SmartText selection does not work with Share and text processing items (Translate etc.)
Reported by
ti...@chromium.org,
Jul 20 2017
|
|||||||
Issue descriptionWebView 61.0.3162.0 Google Pixel, Android O OPR1.170623.011 To reproduce: In WebView shell long press on an expandable item, i.e. geographical address. The selection expands properly, but when you try to Share from the menu the resulting text only contains the word that was long pressed. It seems to be triggered by https://codereview.chromium.org/2926643002/ SelectionPopupController.getSelectedText() is used in share(), processText() and search(). Please notice that copy() works fine because it apparently goes to Blink.
,
Jul 24 2017
,
Jul 24 2017
Pedro, would you mind reverting https://codereview.chromium.org/2926643002/ and merging the revert in M61? Thank you.
,
Jul 24 2017
Reverting that CL would cause a regression in crbug.com/729943. I have a fix for this in chromium-review.googlesource.com/c/582332.
,
Jul 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ce3084ce1e1663d52d8aca1806c89fac5c67f03 commit 2ce3084ce1e1663d52d8aca1806c89fac5c67f03 Author: Pedro Amaral <amaralp@chromium.org> Date: Mon Jul 24 22:20:55 2017 Have mLastSelectedText be updated for onSelectionChanged() A previous CL (crrev.com/2926643002) assumed that any selection modification would go through |SelectionPopupController.showSelectionMenu()|. This assumption proved to be false in the case where Smart Select suggests a new selection. This led to |mLastSelectedText| being stale and causing the linked bug. This CL fixes the issue crrev.com/2926643002 was aiming to fix while also keeping |mLastSelectedText| up to date. Bug: 746770 Change-Id: Idb56a77b7a8905f413f17e757f672361c073bf9e Reviewed-on: https://chromium-review.googlesource.com/582332 Reviewed-by: Alexandre Elias <aelias@chromium.org> Commit-Queue: Pedro Amaral <amaralp@chromium.org> Cr-Commit-Position: refs/heads/master@{#489097} [modify] https://crrev.com/2ce3084ce1e1663d52d8aca1806c89fac5c67f03/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
,
Jul 24 2017
timav@, you marked this as RBB but M62 has already branched. If I merge this change into M61 will it make it into the M61 Beta Release?
,
Jul 24 2017
Yes, I believe it should. M61 was branched on July 21, the M62 branch is planned on August 31.
,
Jul 25 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7068d40a927ce85d6c146e28ba66f5889adc729b commit 7068d40a927ce85d6c146e28ba66f5889adc729b Author: Pedro Amaral <amaralp@chromium.org> Date: Tue Jul 25 22:45:56 2017 Have mLastSelectedText be updated for onSelectionChanged() A previous CL (crrev.com/2926643002) assumed that any selection modification would go through |SelectionPopupController.showSelectionMenu()|. This assumption proved to be false in the case where Smart Select suggests a new selection. This led to |mLastSelectedText| being stale and causing the linked bug. This CL fixes the issue crrev.com/2926643002 was aiming to fix while also keeping |mLastSelectedText| up to date. TBR=amaralp@chromium.org (cherry picked from commit 2ce3084ce1e1663d52d8aca1806c89fac5c67f03) Bug: 746770 Change-Id: Idb56a77b7a8905f413f17e757f672361c073bf9e Reviewed-on: https://chromium-review.googlesource.com/582332 Reviewed-by: Alexandre Elias <aelias@chromium.org> Commit-Queue: Pedro Amaral <amaralp@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489097} Reviewed-on: https://chromium-review.googlesource.com/585655 Reviewed-by: Pedro Amaral <amaralp@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#42} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/7068d40a927ce85d6c146e28ba66f5889adc729b/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
,
Jul 27 2017
,
Aug 3 2017
verified on: pixel xl / opr1.170623.015 vs 61.0.3163.28 |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by ti...@chromium.org
, Jul 20 2017