New issue
Advanced search Search tips

Issue 773515 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Selection menu sometimes doesn't get correct coordinates

Project Member Reported by ctzsm@chromium.org, Oct 10 2017

Issue description

It will receive a Rect(0, 0, 0, 0), so the menu is showing at the top of that WebView.

Scrolling up and down won't correct the Rect info.

A known issue before  issue 763201  and issue 703884 being fixed.
 
Could you please be more specific? What is receiving the zero rect? When is this triggered? Is it consistently reproducing?
Owner: ctzsm@chromium.org
Status: Assigned (was: Available)

Comment 3 by ctzsm@chromium.org, Oct 12 2017

Owner: amaralp@chromium.org
Yeah, so there is a race condition.

If you have an insertion handle, and try to do selection, this might happen. Because sometimes |INSERTION_HANDLE_CLEARED| happened right after showSelectionMenu() and before ActionMode calls onGetContentRect(), so the coordinates got cleared.
This bug also effects Chrome. Are you sure that scrolling won't correct the Rect info? Maybe this changed since Oct 10, but now scrolling does result in the correct position.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e664dd6c6c6509c89fe3acd934e6d1f9e0f7056

commit 1e664dd6c6c6509c89fe3acd934e6d1f9e0f7056
Author: Pedro Amaral <amaralp@chromium.org>
Date: Wed Jan 24 23:09:43 2018

Only clear insertion handle if there is no range selection

There is a race condition where a user makes an insertion handle
and then long-presses some text to make a selection. This can result
in the |INSERTION_HANDLE_CLEARED| coming after the call to
|SelectionPopupControllerImpl#showSelectionMenu()| which resets the
menu position set by |showSelectionMenu()|. This CL fixes that by first
making sure there is no range selection before trying to clear the
insertion handle.

Bug:  773515 
Change-Id: I16b1e845b3960420f424dd04d418464a26bc5f1b
Reviewed-on: https://chromium-review.googlesource.com/884508
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531737}
[modify] https://crrev.com/1e664dd6c6c6509c89fe3acd934e6d1f9e0f7056/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java

Comment 6 by ctzsm@chromium.org, Jan 24 2018

Scrolling will correct the Rect info now, I believe it didn't though.
Status: Fixed (was: Assigned)
Labels: Merge-Request-65
Status: Started (was: Fixed)
This bug is happening pretty frequently across several device types. I think it is best to merge into M65 since it is so early in the beta cycle.
Project Member

Comment 9 by sheriffbot@chromium.org, Jan 25 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 30 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92e0ce8dd161dc0d679f2853ad3e72d176424ae0

commit 92e0ce8dd161dc0d679f2853ad3e72d176424ae0
Author: Pedro Amaral <amaralp@chromium.org>
Date: Tue Jan 30 00:28:20 2018

Only clear insertion handle if there is no range selection

There is a race condition where a user makes an insertion handle
and then long-presses some text to make a selection. This can result
in the |INSERTION_HANDLE_CLEARED| coming after the call to
|SelectionPopupControllerImpl#showSelectionMenu()| which resets the
menu position set by |showSelectionMenu()|. This CL fixes that by first
making sure there is no range selection before trying to clear the
insertion handle.

TBR=amaralp@chromium.org

(cherry picked from commit 1e664dd6c6c6509c89fe3acd934e6d1f9e0f7056)

Bug:  773515 
Change-Id: I16b1e845b3960420f424dd04d418464a26bc5f1b
Reviewed-on: https://chromium-review.googlesource.com/884508
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531737}
Reviewed-on: https://chromium-review.googlesource.com/891657
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#163}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/92e0ce8dd161dc0d679f2853ad3e72d176424ae0/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java

Status: Fixed (was: Started)

Sign in to add a comment