Description: When the cursor is on the To field and the user selects text in the message body, the text will highlight but the selection carets are wrongly positioned. ### Repro Steps: 1. Open Messages app 2. Click pencil icon on the upper-right corner to create a new message 3. Type characters in 10+ rows at least in the message body so that the scroll bar will show up, then scroll down to the bottom of message body 4. Tap on To field to move focus 5. Long tap to select a word in message body 6. Focus is changed to message body, which triggers word suggestion bar popping out and a little bit scrolling. ### Expected: 1. Selection carets should be updated to the right positions along with selected word. ### Actual: 1. Selection carets are not updated to the right positions after scrolling. ### Reproduce rate always Note: This issue occurs after we apply the patch for Bug-1110917, since the patch fixes the focus change problem and do change focus from To field to message body.

If ScrollPositionChanged is called under APZ-enable mode, but APZ is not started yet, ScrollPositionChanged should update positions of selection carets by itself.

Comment on attachment 8547880 [details] [diff] [review] Part1: handle scroll position change that does not triggered by APZ v1 Review of attachment 8547880 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/SelectionCarets.cpp @@ +1219,5 @@ > + } else { > + // TODO: When async pan zoom is enabled, we still need to handle the scroll > + // position changed that does not trigger by APZ. Otherwise, the position of > + // SelectionCarets might be wrong. For example, in SMS app, long press on To > + // field, then long press on the editing field. I think this TODO is fulfilled, and should be removed. It will be even better to rephrase it and put it into the commit message.

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #3) Thanks for the advice. I'll remove it and upload patch v2.

Remove TODO comment that has already been fulfilled in this patch.

try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc03e985e3a3

Comment on attachment 8547919 [details] [diff] [review] Part1: handle scroll position change that is not triggered by APZ v2 Review of attachment 8547919 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/SelectionCarets.cpp @@ +77,5 @@ > , mActiveTouchId(-1) > , mCaretCenterToDownPointOffsetY(0) > , mDragMode(NONE) > , mAsyncPanZoomEnabled(false) > + , mAsyncPanZoomStarted(false) Call it mInAsyncPanZoomGesture @@ +1216,5 @@ > > + SELECTIONCARETS_LOG("Launch scroll end detector"); > + LaunchScrollEndDetector(); > + } else { > + if(!mAsyncPanZoomStarted) { Space before (

Rename flag to mInAsyncPanZoomGesture and add a space between "if" and "(".

This bug can only be reproduced under APZ-enable mode. So, I write a Gaia JS integration test case, which can be run on device with APZ-enable mode.

Nice!

Triage: feature blocker

Comment on attachment 8549448 [details] [diff] [review] Part1: handle scroll position change that is not triggered by APZ v3 (carry r+: roc) 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: Selection carets may be wrongly positioned, so users may not drag the carets to change selection range. Testing completed: Risk to taking this patch (and alternatives if risky): none String or UUID changes made by this patch:none

Comment on attachment 8555671 [details] Part2: Gaia JS integration test case (PR to master) plz see comments in github. some nits need your help to modify before landing.

(In reply to George Duan [:gduan] [:喬智] from comment #15) > Comment on attachment 8555671 [details] > Part2: Gaia JS integration test case (PR to master) > > plz see comments in github. some nits need your help to modify before > landing. I've modified the patch and push it on github. Please check again, thx :)

Enlarge scrollTop offsets to raise robustness. Modify test assertion criterion to compare the selectedContent before/after draging selection caret.

Comment on attachment 8556314 [details] Part2: Gaia JS integration test case v2 (PR to master) LGTM, r=gduan

Remove a trailing whitespace.

Master: https://github.com/mozilla-b2g/gaia/commit/45475198737a504d81932a9c90002902054fce23

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0675625fe7dd The test needs rebasing for v2.2 uplift (if you want it there at all).

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #21) > https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0675625fe7dd > > The test needs rebasing for v2.2 uplift (if you want it there at all). Thx for reminding. We're not planning to put the test in v2.2.

This bug has been verified as "pass" on latest Nightly build of Flame master by the STR in Comment 0. Actual results: Selection carets will be updated to the right positions along with selected. word. See attachment: verified_master.3gp Reproduce rate: 0/10 Device: Flame master (Verified) Build ID 20150705160206 Gaia Revision dc6c18c0dea7af3c40bfff86c530fd877d899dc4 Gaia Date 2015-07-04 01:35:20 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/136c41fca853 Gecko Version 42.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150705.193055 Firmware Date Sun Jul 5 19:31:07 EDT 2015 Bootloader L1TC000118D0