Closed Bug 1110917 Opened 5 years ago Closed 5 years ago

[Text Selection] [Email] CCP menu does not display when selecting text in the body of an email while focus is on another field

Categories

(Core :: DOM: Selection, defect, P2)

37 Branch
ARM
Gonk (Firefox OS)
defect

Tracking

()

VERIFIED FIXED
mozilla38
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
b2g-v2.2 --- fixed
b2g-master --- verified

People

(Reporter: smiko, Assigned: jeremychen)

References

()

Details

(Whiteboard: [2.2-Daily-Testing])

Attachments

(4 files, 5 obsolete files)

Attached file logcat (obsolete) —
Description: When the cursor is on the subject line and the user selects text in the body, the text will highlight but the CCP menu does not display. 

Repro Steps:
1: Update a Flame to 0141212040206.
2: Open Email > Compose
3: Type text into the body of the email.
4: Tap on the subject line to move focus.
5: Long tap on the text in the body.

Actual: Text highlights but CCP menu does not appear.

Expected: CCP menu displays. 

Environmental Variables:
Device: Flame 2.2
BuildID: 20141212040206
Gaia: 1d9ae9cca415ad093beba9521c429350e1f2b14d
Gecko: 5288b15d22de
Gonk: 263b5f41f7733c5577fb101eb4dc8ac5c11cfa8d
Version: 37.0a1 (2.2)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Repro frequency: 5/5

Notes: 
1: This issue does NOT occur in the calendar app.
Actual result: The CCP menu displays while focus in another field.

See attached: logcat 

Video clip: http://youtu.be/I66aI7s2uV8
QA Whiteboard: [QAnalyst-Triage?]
This issue does NOT occur on Flame 2.1 (319mb/full flash) as the Copy/Paste feature does not exist. 

Environmental Variables:
Device: Flame 2.1 (319mb) (Kitkat Base)(Full Flash)
Build ID: 20141212001205
Gaia: 97873dca486abf4162a3345e71b375806937bdec
Gecko: 9faa165ac85d
Gonk: 263b5f41f7733c5577fb101eb4dc8ac5c11cfa8d
Version: 34.0 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Flags: needinfo?(pbylenga)
Asking for guidance from the Core, Selection component, since based on bug 1023688 that component has been involved with these sorts of positioning issues.

Email itself is not in control of the placement of the menu, but happy to work on an HTML/CSS fix in email if there is something extra we need to do other than just enable text selection.

The body area is a contenteditable area, which might be contributing to the issue. It would be difficult to move away from that though for email composition.
Component: Gaia::E-Mail → Selection
Product: Firefox OS → Core
Target Milestone: --- → mozilla37
Version: unspecified → 37 Branch
NI on QA Owner for nomination decision, not a regression.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga) → needinfo?(gchang)
NI developer for this issue.
Flags: needinfo?(gchang) → needinfo?(gduan)
Hi Peter,
I think it's probably a gecko issue. Please check 00:16~00:17, the cursor is still in subject column, we should not allow content in email selectable.
Flags: needinfo?(gduan) → needinfo?(pchang)
Omega, (In reply to George Duan [:gduan] [:喬智] from comment #5)
> Hi Peter,
> I think it's probably a gecko issue. Please check 00:16~00:17, the cursor is
> still in subject column, we should not allow content in email selectable.
George, which part is caused the problem? The caret is remained in subject filed or the text dialog isn't displayed.
Flags: needinfo?(pchang) → needinfo?(gduan)
The following one is the selectioncaret log when problem happened.
The word is selected but it is treated as a collapsed selection event. This may caused by the focus remained in subject filed.

I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SetVisibility:294 : Set visibility hidden, same as the old one
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): HandleEvent:265 : SelectWord from APZ
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): NotifySelectionChanged:1085 : aSel (b30f2580), Reason=3
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SetVisibility:294 : Set visibility hidden, same as the old one
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): NotifySelectionChanged:1085 : aSel (b30f2580), Reason=3
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SetVisibility:294 : Set visibility hidden, same as the old one
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): NotifySelectionChanged:1085 : aSel (b30f2580), Reason=0
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SetVisibility:294 : Set visibility hidden, same as the old one
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SelectWord:603 : Frame=Text(2)"Jcndndnd"@b2d41ed8, ptInFrame=(2540, 740)
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): NotifySelectionChanged:1085 : aSel (b03ff280), Reason=4
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): UpdateSelectionCarets:445 : Selection is collapsed!
I/PRLog   (26635): -1225223852[b6a4a080]: SelectionCarets (b300d3d0): SetVisibility:294 : Set visibility hidden, same as the old one
Flags: needinfo?(gduan)
Please help to check the focus call in SelectionCarets::SelectWord.
Flags: needinfo?(tlin)
Priority: -- → P2
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][textselection]
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Flags: needinfo?(tlin)
Assignee: tlin → jeremychen
Refactor _long_press_to_select_first_word into two separated functions to isolated the side effect which causes focus changed. 

Implement focus changing tests.
Attachment #8535712 - Attachment is obsolete: true
Attachment #8541433 - Attachment is obsolete: true
Use focusable instead of editable to make sure the focus changes correctly.
Attachment #8542850 - Attachment is obsolete: true
Blocks: 1098161
No longer blocks: 1098161
Attachment #8543789 - Flags: review?(mdas) → review+
Attachment #8542849 - Flags: review?(mdas) → review+
Comment on attachment 8547476 [details] [diff] [review]
Part 2: Fix focus not changing while selecting text by long press. r=roc (v2, carry r+)

Approval Request Comment
[Feature/regressing bug #]: Text selection/copy/paste support on B2G
[User impact if declined]: User may not use CCP feature in Email text body
[Describe test coverage new/current, TBPL]: tested manually
[Risks and why]: Bug 1120358 is revealed since focus does not change correctly before
[String/UUID change made/needed]: none
Attachment #8547476 - Flags: approval-mozilla-aurora?
Comment on attachment 8543789 [details] [diff] [review]
Part 3: Remove HTMLElement.location usage (API change) v1

Approval Request Comment
[Feature/regressing bug #]: marionette test suite
[User impact if declined]: 
[Describe test coverage new/current, TBPL]: tested manually
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8543789 - Flags: approval-mozilla-aurora?
Comment on attachment 8542849 [details] [diff] [review]
Part 1: Add test cases for focus change v1

Approval Request Comment
[Feature/regressing bug #]: marionette test suite
[User impact if declined]: focus change bugs may happen if we can't test them in advance
[Describe test coverage new/current, TBPL]: tested manually
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8542849 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1121308
Depends on: 1121534
Jeremy - this is causing perma-failures on some gaia integration tests which unfortunately have just been hidden from treeherder. Just wondering if you could back this out to re-open gaia, or possibly help us investigate the failures?

From a brief look, it seems that we're seeing some different behavior now during a long press in b2g desktop. I'm not sure if this manifests on the device yet.
Flags: needinfo?(jeremychen)
Sorry Jeremy, I had to back this out to get our tests green again.

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/57c5119c5da0
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/bcd975637382
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/0529ed06c7bf
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
After my investigation, I found that Perma-fails on Gij3 was caused by b2g crashing. A null check is now added to avoid sending nullptr into nsContentUtils::HasNonEmptyTextContent. This should prevent b2g from crashing. I've passed b2g-desktop Gij3 test locally, and I'm waiting for positive result from try server.
Attachment #8547476 - Attachment is obsolete: true
Attachment #8547476 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jeremychen)
Comment on attachment 8550132 [details] [diff] [review]
Part 2: Fix focus not changing while selecting text by long press. r=roc (v3, carry r+)

Approval Request Comment
[Feature/regressing bug #]: Text selection/copy/paste support on B2G
[User impact if declined]: User may not use CCP feature in Email text body
[Describe test coverage new/current, TBPL]:
[Risks and why]: Bug 1120358 is revealed since focus does not change correctly before
[String/UUID change made/needed]: none
Attachment #8550132 - Flags: approval-mozilla-aurora?
Comment on attachment 8542849 [details] [diff] [review]
Part 1: Add test cases for focus change v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): marionette test suite
User impact if declined: focus change bugs may happen if we can't test them in advance
Testing completed: 
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #8542849 - Flags: approval-mozilla-aurora? → approval-mozilla-b2g37?
Comment on attachment 8543789 [details] [diff] [review]
Part 3: Remove HTMLElement.location usage (API change) v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): marionette test suite
User impact if declined: none
Testing completed: 
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #8543789 - Flags: approval-mozilla-aurora? → approval-mozilla-b2g37?
Comment on attachment 8550132 [details] [diff] [review]
Part 2: Fix focus not changing while selecting text by long press. r=roc (v3, carry r+)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Text selection/copy/paste support on B2G
User impact if declined: User may not use CCP feature in Email text body
Testing completed: 
Risk to taking this patch (and alternatives if risky): Bug 1120358 is revealed since focus does not change correctly before
String or UUID changes made by this patch: none
Attachment #8550132 - Flags: review+
Attachment #8550132 - Flags: approval-mozilla-b2g37?
Attachment #8550132 - Flags: approval-mozilla-aurora?
Keywords: verifyme
Attachment #8542849 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Attachment #8543789 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Attachment #8550132 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
The problem is verified not happen on latest Flame 2.2 & 3.0 build.

Steps:
1: Update a Flame to 0141212040206.
2: Open Email > Compose
3: Type text into the body of the email.
4: Tap on the subject line to move focus.
5: Long tap on the text in the body.

Actual: Both text highlight and CCP menu display and work normally.

Fail rate:0/5
See attachment:907.mp4

Flame 2.2 version:
Build ID               20150203002504
Gaia Revision          cd62ff9fe199fb43920ba27bd5fdbc5c311016fc
Gaia Date              2015-02-03 00:56:43
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/11d93135c678
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150203.041704
Firmware Date          Tue Feb  3 04:17:15 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 version:
Build ID               20150203055658
Gaia Revision          ae5a1580da948c3b9f93528146b007fc4f6a712b
Gaia Date              2015-02-02 19:50:21
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/ae5d04409cd9
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150203.093120
Firmware Date          Tue Feb  3 09:31:32 EST 2015
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+][textselection] → [QAnalyst-Triage+][textselection][MGSEI-Triage+]
Attached video 907.MP4
QA Whiteboard: [QAnalyst-Triage+][textselection][MGSEI-Triage+] → [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+]
Verified fixed on these Builds

Environmental Variables:
Device: Flame 2.5
BuildID: 20151001030229
Gaia: bd8ff00faac97ad6a2df5a6217910b8d295d56a3
Gecko: 096c0f407f8ba3ef7cfe4e0b831761993cac38b1
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 44.0a1 (2.5) 
Firmware Version: v18D

Environmental Variables:
Device: Aries 2.5
BuildID: 20151001114357
Gaia: bd8ff00faac97ad6a2df5a6217910b8d295d56a3
Gecko: 2c1fb007137dcb68b1862a79553b53f1a34c99c3
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf



Bug Repros on this Build

Device: Flame 2.2
BuildID: 20150108010221
Gaia: d4dac29613076bdba3cb8adc217deadb08a2ac20
Gecko: 70de2960aa87
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2) 
Firmware Version: v18D
Status: RESOLVED → VERIFIED
Flags: needinfo?(jmercado)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+] → [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+] [failed-verification]
Correction: Does not repro on the most recent Flame 2.2 build

Bug does not repro on this build
Environmental Variables:
Device: Flame 2.2
BuildID: 20151001032503
Gaia: 5dd95cfb9f1d6501ce0e34414596ef3dd9c2f583
Gecko: 5a8574453182
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D
QA Whiteboard: [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+] [failed-verification] → [QAnalyst-Triage+][COM=Text Selection][MGSEI-Triage+]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.