[SMS] [Text Selection] Long pressing on attached files will cause blue text-selection carets to appear around or under the file preview

VERIFIED FIXED in 2.2 S10 (17apr)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: jmitchell, Assigned: steveck)

Tracking

unspecified
2.2 S10 (17apr)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [3.0-Daily-Testing], URL)

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Posted image 4CaretGoat.png
Description:
In the messages app when you add an attachment it will add a preview / icon of the attachment. When you press on this it will cause a pair of blue text selection carets to appear. Sometimes you can get multiple sets of carets from multiple long-presses. These carets will also appear on the attachement context menu (shown in video) but I would consider that a bug-of-a-bug at this point. 

example: 
Picture: A set of blue carets will appear in 1 of two places. Sometimes below the picture in line with the left and right side of the picture. Sometimes it will appear near the top of the picture - over it, but the carets are truncated for the half that would expand past the boundary of the picture (see screenshot). These can both appear at the same time


Repro Steps:
1) Update a Flame to 20150408010203
2) Launch Messages 
3) Create new message
4) Add a variety of attachments
5) Long press on some of the attachments


Actual:
Blue text selection carets appear

Expected:
No Blue text selections will appear 



Environmental Variables:
Device: Flame 3.0
Build ID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0



Repro frequency: 9/10
See attached: Screenshot, Video Clip: http://youtu.be/V3nj2kx7VYA
(Reporter)

Comment 1

4 years ago
This issue also occurs in Flame KK 2.2  (Text Selection was not implemented prior to this)

Device: Flame 2.2 (KK - Nightly - Full Flash - 319mem)
Build ID: 20150408002503
Gaia: ea735c21bfb0d78333213ff0376fce1eac89ead6
Gecko: 43041c78052b
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?][COM=Text Selection]
Flags: needinfo?(pbylenga)
Hey Jeremy, maybe you can help diagnosing the issue here?
Flags: needinfo?(jeremychen)
QA Whiteboard: [QAnalyst-Triage?][COM=Text Selection] → [QAnalyst-Triage+][COM=Text Selection]
Flags: needinfo?(pbylenga)
It looks like sms app creates an iframe for each attachment when editing a message. After first long press to select text (through the context menu), we select the whole iframe. But, the second long press (long press on attachment again) would select the div element for attachment inside the iframe w/o clearing the outer carets (carets for iframe). Accordingly, more than two carets appear on the screen. 

It's an initial investigation so far. I'll dig a little deeper later.
Flags: needinfo?(jeremychen)
Duplicate of this bug: 1153171
I think this bug could be fixed by setting the attachment user-select to none. I've tested locally and it worked. Please see bug 1152263 comment 7.

Hi Steve, since you've already made a patch, I think maybe you could take this bug and land your patch here.
Flags: needinfo?(schung)
(Assignee)

Comment 6

4 years ago
I could not reproduce the duplicate caret for attachment, if the main purpose of this bug is to exclude the attchments in the selection, maybe we can set dup to Bug 1127648. Hi Omega, do you think it would be better to do so after v2.2?

Hi Jeremy, even we set the attachment to select: none, the file name inside the attachment and some wierd space will still hightlight while selection(text won't be copied), do you have any idea about this?
Flags: needinfo?(schung)
Flags: needinfo?(ofeng)
Flags: needinfo?(jeremychen)
(In reply to Steve Chung [:steveck] from comment #6)
> I could not reproduce the duplicate caret for attachment, if the main
> purpose of this bug is to exclude the attchments in the selection, maybe we
> can set dup to Bug 1127648. Hi Omega, do you think it would be better to do
> so after v2.2?
> 
> Hi Jeremy, even we set the attachment to select: none, the file name inside
> the attachment and some wierd space will still hightlight while
> selection(text won't be copied), do you have any idea about this?

Hi Steve, the duplicate caret happens when editing a message with attachment, not for message already sent in the dialog stream. In the editing mode, attachment would be embedded as an iframe. So, we can select the iframe first, then select the attachment inside the iframe to cause the duplicate carets. After we send the message, the dom structure would be changed and the iframe for attachment is no longer exist. I guess this is why you can't reproduce this bug.

Accordingly, I think this bug is a bit different from Bug 1127648. Simply setting the attachment to select: none can fix this bug, but not Bug 1127648. If we want to keep the attachment to be selectable for now, I think another approach could be only setting the iframe to select: none.

For the weird space you mentioned, I think those are text node with pure newline ("\n") or pure whitespace ("       ") that created by sms app. As we've discussed on Bugzilla before, we can't prevent select: none elements from selection API, so those text would be highlight when press the select text from context menu.
Flags: needinfo?(jeremychen)
> Accordingly, I think this bug is a bit different from Bug 1127648. Simply
> setting the attachment to select: none can fix this bug, but not Bug
> 1127648. If we want to keep the attachment to be selectable for now, I think
> another approach could be only setting the iframe to select: none.

Here is what I said about the approach.
I've tested locally by setting -moz-user-select: none in attachment-draft css rule instead of in attachment-container css rule. This could fix the duplicate caret bug by avoid selecting attachment draft in an iframe. Moreover, this also ensures that an attachment can still be selectable in both sms draft editing and sms dialog stream, which I assume is the UI we don't want to change for now. Would it be a better approach?
Flags: needinfo?(schung)
(Assignee)

Comment 10

4 years ago
Comment on attachment 8592149 [details] [review]
[gaia] steveck-chung:message-selection > mozilla-b2g:master

Hi Jeremy, sorry I misunderstood the report description and you're right that set selection none to attachment did solve problems for message compose area. Although the existing message thread might still have another issue but we can track this in Bug 1127648, wdyt?

Hi Julien, it's a simple css patch that set user-select none to attachment. Since we still have Bug 1127648 for tracking thread selection part, it should be fairly enough for fixing the issues inside composer.
Flags: needinfo?(schung)
Attachment #8592149 - Flags: review?(felash)
Comment on attachment 8592149 [details] [review]
[gaia] steveck-chung:message-selection > mozilla-b2g:master

r=me, seems to fix the issue and this makes sense
Attachment #8592149 - Flags: review?(felash) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Assignee: nobody → schung
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Clear ni for me since this bug is resolved.
Flags: needinfo?(ofeng)
Hi Steve, since v2.2 is affected too, would you uplift the patch?
Flags: needinfo?(schung)
(Assignee)

Comment 15

4 years ago
Hi Wesley, I nominated this bug as 2.2 blocker. Do we still have 2.2 triage now or I should ask for approval directly?
blocking-b2g: --- → 2.2?
Flags: needinfo?(schung) → needinfo?(whuang)
Steve, you should ask approval already, so that we don't wait 1 more day to have this approved :)
Looks like a blocker to me. 
We'll have triage in a few hours to determine it. 
Please stay tuned.
triage: major issue on new feature. 
please go ahead and ask for approval to uplift.
blocking-b2g: 2.2? → 2.2+
Flags: needinfo?(whuang)
(Assignee)

Comment 19

4 years ago
Comment on attachment 8592149 [details] [review]
[gaia] steveck-chung:message-selection > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):Test selection feature
[User impact] if declined: Bad user experience with weird caret display while selecting in composer area with attachment contained inside
[Testing completed]: N/A
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A
Attachment #8592149 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8592149 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
This issue is verified fixed on Flame Master and 2.2.

Result: Long-pressing an attached file does not bring the text selection carets. 

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150422010202
Gaia: 15134b080b5f406e5aa36f5136c17dafb4e31f64
Gecko: 946ac85af8f4
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150422002505
Gaia: 41a85c5f9db291d4f7c0e94c8416b5115b4ee407
Gecko: a87a05e7d0ef
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][COM=Text Selection] → [QAnalyst-Triage?][COM=Text Selection]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][COM=Text Selection] → [QAnalyst-Triage+][COM=Text Selection]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.