1121335 - [Text Selection] Caret can't be dragged freely in email compose view

Status: VERIFIED FIXED

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

Description

[Teri Wen [:twen]]

### STR
0. Prerequisite: Set up an email account.
1. Open email app.
2. Tap pencil icon to compose a new email.
3. Enter some message with four lines of text. 
4. Long press on the message to bring out copy/paste selection menu.
5. Tap select all.
6. Drag right caret to the previous line.
 

### Actual
Right caret will not move past the beginning of the last line

### Expected
Right caret can be dragged to the previous line

### Version
Gaia-Rev        7c5b27cad370db377b18a742d3f3fdb0070e899f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/f5947d58ab02
Build-ID        20150112160202
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  65
FW-Date         Mon Dec 15 18:51:29 CST 2014
Bootloader      L1TC000118D0

Comment 1

[Peter Chang[:pchang]]

This is related to the selection range issue. Assign to myself.

Comment 2

[Peter Chang[:pchang]]

I found this issue was caused by two problems, listed in the following.

a. Email app assigns -moz-user-selection:none for all elements by default, and change to -moz-user-select:text if the element is content editable. Under this configuration, the <br> becomes unselectable. Therefore, it introduces multiple selection ranges in compose view if users input text as multiple lines. It causes some trobule during the multiple lines copying because the <br> is not selectable.

b. When multiple select ranges happen, there exists wrong selection range input for checking the valid selection drag in [1]


[1]https://dxr.mozilla.org/mozilla-central/source/layout/base/SelectionCarets.cpp?from=SelectionCarets.cpp&case=true#

Comment 3

[Peter Chang[:pchang]]

Update the URL which can reproduce this issue in browser.

Comment 4

[Peter Chang[:pchang]]

Attachment: choose the right selection range for drag mode

Comment 5

[Peter Chang[:pchang]]

Attachment: Add the testing of selectioncarets drag with multiple selection ranges

Comment 6

[Peter Chang[:pchang]]

Try is looking good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a2a4687ef9f

Comment 7

[Carsten Book [:Tomcat]]

Hi Peter,

the 2 patch failed to apply:

Which patches do you want to import? [Default is '1', use '1-2' for all] 2
adding 1121335 to series file
renamed 1121335 -> bug-1121335-test.patch
applying bug-1121335-test.patch
patching file layout/base/tests/marionette/test_selectioncarets.py
Hunk #2 FAILED at 33
Hunk #3 FAILED at 97
2 out of 4 hunks FAILED -- saving rejects to file layout/base/tests/marionette/test_selectioncarets.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug-1121335-test.patch

could you take a look thanks!

Comment 8

[Peter Chang[:pchang]]

Attachment: choose the right selection range for drag mode

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 #):
User impact if declined: The drag behavior in email compose view will have problem
Testing completed: Locally on dog-food device and add the test case to verify this patch
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Comment 9

[Peter Chang[:pchang]]

Attachment: Add the testing of selectioncarets drag with multiple selection ranges

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: This is the test case for attachment 8551083 [details] [diff] [review]
Testing completed: Verify the test case passed in local
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: none

Comment 10

[Peter Chang[:pchang]]

(In reply to Carsten Book [:Tomcat] from comment #7)
> Hi Peter,
> 
> the 2 patch failed to apply:
> 
> Which patches do you want to import? [Default is '1', use '1-2' for all] 2
> adding 1121335 to series file
> renamed 1121335 -> bug-1121335-test.patch
> applying bug-1121335-test.patch
> patching file layout/base/tests/marionette/test_selectioncarets.py
> Hunk #2 FAILED at 33
> Hunk #3 FAILED at 97
> 2 out of 4 hunks FAILED -- saving rejects to file
> layout/base/tests/marionette/test_selectioncarets.py.rej
> patch failed, unable to continue (try -v)
> patch failed, rejects left in working dir
> errors during apply, please fix and refresh bug-1121335-test.patch
> 
> could you take a look thanks!

 Carsten, I just update the patch again, please help to apply again. Thanks.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cbaffb79715
https://hg.mozilla.org/integration/mozilla-inbound/rev/c22bfc1c641c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Backed out for Marionette failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/df41627723a9

https://treeherder.mozilla.org/logviewer.html#?job_id=5826789&repo=mozilla-inbound

Comment 13

[Peter Chang[:pchang]]

Attachment: v3 Add the testing of selectioncarets drag with multiple selection ranges

Fix the marionette fail because of test_selectioncarets.py merge conflict.

Comment 14

[Peter Chang[:pchang]]

Local marionette was passed.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a763a5642590
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f52e0321f3c

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a763a5642590
https://hg.mozilla.org/mozilla-central/rev/5f52e0321f3c

Comment 17

[Peter Chang[:pchang]]

Comment on attachment 8554351 [details] [diff] [review]
v3 Add the testing of selectioncarets drag with multiple selection ranges

Review of attachment 8554351 [details] [diff] [review]:
-----------------------------------------------------------------

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: This is the test case for attachment 8551083 [details] [diff] [review] [diff] [review]
Testing completed: Verify the test case passed in local
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: none

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7bb131b4d694
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1e2bb8d9fcde

Comment 19

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Flame 2.2 and 3.0. After tapping to select all text, user is able to move the right caret freely in email compose field.

The left caret, however, is difficult to get it to start moving, if user has edge gesture feature enabled (it is enabled by default), because the left caret is so close to the edge of the screen. It is irrelevant to this bug but I'm just putting what I observed here.

Device: Flame 3.0
BuildID: 20150511010202
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: d8420a541d1c
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150511002500
Gaia: 528ef60e7cda09ad43478065f5d33bda398fbeb7
Gecko: 8d04cc085cf5
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0