Issue metadata
Sign in to add a comment
|
Extending selection back and forward following double click doesn't keep original base. |
||||||||||||||||||||||
Issue descriptionChrome Version: 62.0.3165.0 (Official Build) canary (64-bit) OS: all (verified on macOS and Windows) What steps will reproduce the problem? (1) Go to any web page allowing text selection, such as https://chromium-review.googlesource.com/584037/ (2) Double-click the word "Allow" in "584037: Allow a few..." (3) Move the mouse to the right to try to highlight the entire CL description (4) Move the mouse up a bit "by accident" so that text selection goes in the opposite direction (5) Move the mouse back down and select to the end of the CL description ("...before losing the context.") What is the expected result? Expect the entire CL description ("Allow a few SwapBuffers failures on Android before losing the context.") to be selected. What happens instead? The first word of the selection is lost. (" a few SwapBuffers failures on Android before losing the context.") This is a recent regression in Canary as of roughly the past week. Sorry I didn't file it earlier but I didn't understand what was going on until now. It affects all text selection in the browser and is a major regression. It doesn't appear to have been filed yet. The beginning of the selection should remain at the beginning of the first word that was double-clicked ("Allow") and not move around. Each time the selection direction changes in multi-word text selection, the anchor jumps forward one word.
,
Jul 28 2017
,
Jul 28 2017
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone. All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 28 2017
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da0e8f7a07e227aa011a3259fca40492575c4177 commit da0e8f7a07e227aa011a3259fca40492575c4177 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Tue Aug 01 05:15:13 2017 Introduce ComputeStartFromEndForExtendForward() for ExtendSelectionAsDirectional() This patch introduces |ComputeStartFromEndForExtendForward()| for |ExtendSelectionAsDirectional()| as replacement of |ComputeStartRespectingGranularity()| to use correct start position when extending selection with word or paragraph granularity. Note: When passing end of word/pargraph position to |ComputeStartRespectingGranularity()|, it returns next word/paragraph position instead of start of word/paragraph of the word. Behavior of before this patch: abc |def^ ghi* => abc def ^ghi| After this patch: abc |def^ ghi* => abc ^def| ghi where "^" is base, "|" is extent, "*" is shift-click or drag position. Bug: 748361 Change-Id: I3dc6ea158077121cda24bb0787895d4d5561b2a3 Reviewed-on: https://chromium-review.googlesource.com/590838 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#490856} [add] https://crrev.com/da0e8f7a07e227aa011a3259fca40492575c4177/third_party/WebKit/LayoutTests/editing/selection/mouse/extend_by_word_with_base_is_end.html [modify] https://crrev.com/da0e8f7a07e227aa011a3259fca40492575c4177/third_party/WebKit/Source/core/editing/SelectionController.cpp
,
Aug 1 2017
,
Aug 1 2017
Thanks for your quick fix! Since this appeared to be a regression, could you block this bug on the one which introduced it so they can more easily be found? Did the fix for Issue 692923 introduce this? Thanks.
,
Aug 2 2017
,
Aug 3 2017
,
Aug 3 2017
This was broken in 61.0.3154.0. This needs to be merged to M61.
,
Aug 4 2017
Add Merge-Request-61 per #c10
,
Aug 4 2017
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review Please contact the 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
,
Aug 4 2017
Before we approve merge to M61, please answer followings: * Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61? * Any other important details to justify the merge.
,
Aug 7 2017
This is a major regression in text selection. People will see subtly wrong behavior when they attempt to highlight text for selection on all web pages. In my opinion it is a must-fix for M61. yosin@ should comment on the risk of https://chromium-review.googlesource.com/590838 . It looks like a small and self-contained fix to me, and the new automated tests look solid.
,
Aug 7 2017
Thank you kbr@. Approving merge to M61 branch 3163 based on comment #14. Yosin@, if you think cl is risky, please do not merge.
,
Aug 7 2017
,
Aug 8 2017
Thanks kbr@ follow-up! I think my patch is not risky. Going to merge to M61.
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95cb41bc2dd990717f56d7659431c0dc74c70464 commit 95cb41bc2dd990717f56d7659431c0dc74c70464 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Tue Aug 08 06:12:40 2017 Introduce ComputeStartFromEndForExtendForward() for ExtendSelectionAsDirectional() This patch introduces |ComputeStartFromEndForExtendForward()| for |ExtendSelectionAsDirectional()| as replacement of |ComputeStartRespectingGranularity()| to use correct start position when extending selection with word or paragraph granularity. Note: When passing end of word/pargraph position to |ComputeStartRespectingGranularity()|, it returns next word/paragraph position instead of start of word/paragraph of the word. Behavior of before this patch: abc |def^ ghi* => abc def ^ghi| After this patch: abc |def^ ghi* => abc ^def| ghi where "^" is base, "|" is extent, "*" is shift-click or drag position. TBR=yosin@chromium.org (cherry picked from commit da0e8f7a07e227aa011a3259fca40492575c4177) Bug: 748361 Change-Id: I3dc6ea158077121cda24bb0787895d4d5561b2a3 Reviewed-on: https://chromium-review.googlesource.com/590838 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#490856} Reviewed-on: https://chromium-review.googlesource.com/604820 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#377} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [add] https://crrev.com/95cb41bc2dd990717f56d7659431c0dc74c70464/third_party/WebKit/LayoutTests/editing/selection/mouse/extend_by_word_with_base_is_end.html [modify] https://crrev.com/95cb41bc2dd990717f56d7659431c0dc74c70464/third_party/WebKit/Source/core/editing/SelectionController.cpp
,
Aug 18 2017
,
Aug 18 2017
Rechecked this issue on Chrome beta version 61.0.3163.49 on Windows 7, 10, MAC 10.12.6, Ubuntu 14.04. Fix is working as intended as the selection of text starts from where the original text was clicked. Adding TE-verified labels. Thanks.! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by yoichio@chromium.org
, Jul 28 2017Status: Assigned (was: Untriaged)
Summary: Extending selection back and forward following double click doesn't keep original base. (was: The anchor in multi-word text selection moves forward a word when the selection direction changes)