Closed Bug 1120013 Opened 8 years ago Closed 7 years ago

[Camera][Text Selection] Camera app to follow text selection pattern - Long pressing camera icons / menu selections results in text selection pop-ups (Blue Carets and copy/paste icons)

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: jmitchell, Assigned: gduan)

References

Details

(Whiteboard: [2.2-Daily-Testing])

Attachments

(2 files)

Description:
 In the Camera app if you long-press on any of of the icons (flash, front-face, Options) you will see a set of blue carets around the icon and in the options menu you will see copy/paste icons over the options.


Repro Steps:
1) Update a Flame to 20150109010206
2) Launch Camera
3) Long press on icons and option menu options

Actual:
text selection icons appear

Expected:
camera app will follow text selection options
* Input fields (input areas, input dialogs and search fields) are the only Building Blocks with selectable text.

Environmental Variables:
Device: Flame Master
Build ID: 20150109010206
Gaia: 5f0dd37917c4a6d8fa8724715d4d3797419f9013
Gecko: b3f84cf78dc2
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (Master)
Firmware Version: V18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0


Repro frequency: 14/15


-------------------------------------------------------------------------

This issue also occurs on 2.2 (V188-1 Base)

Device: Flame Master (KK - Nightly - full Flashed)
Build ID: 20150109010206
Gaia: 5f0dd37917c4a6d8fa8724715d4d3797419f9013
Gecko: b3f84cf78dc2
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (Master)
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0

--------------------------------------------------------------

This issue does NOT occur in 2.1 but I do not think this is a regression from 2.1 but more likely something new due to the text select/copy/paste functionality.

Device: Flame 2.1 (KK - Nightly - Full Flashed)
Build ID: 20150108001214
Gaia: ed2e278753e8c9301ba322dcf2c3591f5928408d
Gecko: 127a0ead5f83
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 34.0 (2.1)
Firmware Version: V18D
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
NI on Component owner for nomination decision and assignment.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga) → needinfo?(gchang)
NI developer to fix this.
Flags: needinfo?(gchang) → needinfo?(gduan)
Attached file PR to master
Hi Wilson,
could you help to review it?
Thanks.
Flags: needinfo?(gduan)
Attachment #8549556 - Flags: review?(wilsonpage)
Comment on attachment 8549556 [details] [review]
PR to master

I first addressed the text-selection issue by placing a `-moz-user-select: none` on the body. I've just found out that this rule will not apply to absolutely positioned descendents (Gecko only) [1].

As part of this patch could you also add this rule to the other absolutely positioned elements in the app. Thanks for your help :)

[1] https://developer.mozilla.org/en-US/docs/Web/CSS/user-select?redirectlocale=en-US&redirectslug=CSS%2Fuser-select#not-spec
Attachment #8549556 - Flags: review?(wilsonpage)
Actually, this rule cannot be inherited if we put it in body or html. 

I think we only need one rule.

* {
  -moz-user-select: none;
}
That would prevent all possible risk. What do you think?
Flags: needinfo?(wilsonpage)
(In reply to George Duan [:gduan] [:喬智] from comment #5)
> Actually, this rule cannot be inherited if we put it in body or html. 
> 
> I think we only need one rule.
> 
> * {
>   -moz-user-select: none;
> }
> That would prevent all possible risk. What do you think?

People often say the `*` selector should be avoided for performance reasons. I'm not qualified enough to say whether that's true or not. I'd say just make sure the selector is on .controls, .hud, .preview-gallery and any absolutely positioned things inside the preview-gallery.
Flags: needinfo?(wilsonpage)
Comment on attachment 8549556 [details] [review]
PR to master

AFAK, there's a bug from gecko. If you put pointer-event: none on a editable field, then selection event will pass to its parent element instead of the element itself. So, if you didn't set moz-user-select: none to its parent, it might have interesting bug to our phone, such as unexpected displaying of caret or bubble.

I would still suggest to put moz-user-select into * to prevent other possible failure.

Patch is updated as u suggested, please kindly check again. Thanks.
Attachment #8549556 - Flags: review?(wilsonpage)
(In reply to George Duan [:gduan] [:喬智] from comment #7)
> I would still suggest to put moz-user-select into * to prevent other
> possible failure.

OK, I think you're right, let's just go with that. I noticed we already have a `*` selector we can throw the rule into. Sorry for wasting your time :(
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][textselection]
Flags: needinfo?(gduan)
Attachment #8549556 - Flags: review?(wilsonpage)
Comment on attachment 8549556 [details] [review]
PR to master

Plz kindly review again, thanks.
Flags: needinfo?(gduan)
Attachment #8549556 - Flags: review?(wilsonpage)
Attachment #8549556 - Flags: review?(wilsonpage) → review+
master: https://github.com/mozilla-b2g/gaia/commit/1461ba4d408f2c9028273c7cb875720c33104654
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
QA Whiteboard: [QAnalyst-Triage+][textselection] → [QAnalyst-Triage+][COM=Text Selection]
verified fixed on 3.0 

Device: Flame 3.0 (KK - Nightly - Full Flash)
Build ID: 20150206010204
Gaia: 94af4b42d2ace6c9f38f31de77240604fac68af1
Gecko: 7c5f187b65bf
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
---------------------------------------------------------------------------------------

Issue is still present on 2.2 

Device: Flame 2.2 (KK - Nightly - Full Flash)
Build ID: 20150206002505
Gaia: a52999ce7f783177deb17e267bf003a53e6fde06
Gecko: 01446d5231ef
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Flags: needinfo?(pbylenga)
NI on component owner for tracking, this issue is still present on 2.2 but is verified for master (3.0).
Status: RESOLVED → VERIFIED
Flags: needinfo?(pbylenga) → needinfo?(gchang)
NI developer to uplift this fix to v2.2 branch.
Flags: needinfo?(gchang) → needinfo?(gduan)
Comment on attachment 8549556 [details] [review]
PR to master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): No.
[User impact] if declined:
Selection bubble on icons and option menu options should be prevented, or it might confuse user.
[Testing completed]: Manual test.
[Risk to taking this patch] (and alternatives if risky): No, it's safe.
[String changes made]:
Flags: needinfo?(gduan)
Attachment #8549556 - Flags: approval-gaia-v2.2?
Attachment #8549556 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Attached video v2-2.3gp
This bug has been verified as pass on latest build of Flame v2.2 by the STR in Comment 0. 

Results:
There is no blue carets around the icon and in the options menu you will not see copy/paste icons over the options after long-press on any of the icons in the Camera app.

Attachment:v2-2.3gp
Rate:0/20

Device: Flame 2.2 (Verified)
Build ID               20150630162500
Gaia Revision          bd386f346eb1591fddbc84bf034b22700e7e2a58
Gaia Date              2015-06-30 15:53:15
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f16c1125b9d6
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150630.200238
Firmware Date          Tue Jun 30 20:02:49 EDT 2015
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+][COM=Text Selection] → [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.