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

Status: VERIFIED FIXED

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

Description

[Seth Miko [:SMiko]]

Attachment: logcat

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

Comment 1

[Seth Miko [:SMiko]]

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

Comment 2

[James Burke [:jrburke]]

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.

Comment 3

[Peter Bylenga [:PBylenga]]

NI on QA Owner for nomination decision, not a regression.

Comment 4

[Gerry Chang [:gchang]]

NI developer for this issue.

Comment 5

[(Inactive after June) George Duan [: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.

Comment 6

[Peter Chang[: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.

Comment 7

[Peter Chang[:pchang]]

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

Comment 8

[Peter Chang[:pchang]]

Please help to check the focus call in SelectionCarets::SelectWord.

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: WIP

Comment 10

[Jeremy Chen ([e-mail])]

Attachment: Part 1: Add test cases for focus change v1

Refactor _long_press_to_select_first_word into two separated functions to isolated the side effect which causes focus changed. 

Implement focus changing tests.

Comment 11

[Jeremy Chen ([e-mail])]

Attachment: Part 2: Add test cases for focus change v1

Comment 12

[Jeremy Chen ([e-mail])]

Attachment: Part 2: Fix focus not changing by long press v1

Use focusable instead of editable to make sure the focus changes correctly.

Comment 13

[jichen]

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ef658b091df

Comment 14

[Jeremy Chen ([e-mail])]

Attachment: Part 3: Remove HTMLElement.location usage (API change) v1

Comment 15

[Jeremy Chen ([e-mail])]

try result (marionette test): https://treeherder.mozilla.org/#/jobs?repo=try&revision=dec1d0bee132

Comment 16

[Jeremy Chen ([e-mail])]

Attachment: Part 2: Fix focus not changing while selecting text by long press. r=roc (v2, carry r+)

Rebased

Comment 17

[Jeremy Chen ([e-mail])]

try result (v2, carry r+): https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d7d2ef3d811

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/58dd19c8bd07
https://hg.mozilla.org/integration/mozilla-inbound/rev/16678c008a72
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d55be972607

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/58dd19c8bd07
https://hg.mozilla.org/mozilla-central/rev/16678c008a72
https://hg.mozilla.org/mozilla-central/rev/7d55be972607

Comment 20

[Jeremy Chen ([e-mail])]

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

Comment 21

[Jeremy Chen ([e-mail])]

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

Comment 22

[Jeremy Chen ([e-mail])]

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

Comment 24

[Kevin Grandon :kgrandon]

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.

Comment 25

[Gregor Wagner [:gwagner]]

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

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/57c5119c5da0
https://hg.mozilla.org/mozilla-central/rev/bcd975637382
https://hg.mozilla.org/mozilla-central/rev/0529ed06c7bf

Comment 27

[Jeremy Chen ([e-mail])]

Attachment: Part 2: Fix focus not changing while selecting text by long press. r=roc (v3, carry r+)

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.

Comment 28

[Jeremy Chen ([e-mail])]

try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=97c3489f9e08&exclusion_state=all

Comment 29

[Jeremy Chen ([e-mail])]

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

Comment 30

[Jeremy Chen ([e-mail])]

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

Comment 31

[Jeremy Chen ([e-mail])]

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

Comment 32

[Jeremy Chen ([e-mail])]

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

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2342e344fb3
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff34718d7f68
https://hg.mozilla.org/integration/mozilla-inbound/rev/33a81cbadb6e

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b2342e344fb3
https://hg.mozilla.org/mozilla-central/rev/ff34718d7f68
https://hg.mozilla.org/mozilla-central/rev/33a81cbadb6e

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7cb5da22dd49
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/63aa06e42413
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d690de91db1

Comment 36

[Lance（leave from Mozilla）]

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

Comment 37

[Lance（leave from Mozilla）]

Attachment: 907.MP4

Comment 38

[Max Murrell [MMurrell]]

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

Comment 39

[Max Murrell [MMurrell]]

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