New issue
Advanced search Search tips

Issue 699903 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Feature



Sign in to add a comment

Selection#modify() with word granularity should not count punctuation as part of word

Reported by szfjoz...@gmail.com, Mar 9 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce the problem:
1. Copy/paste any text with symbol and characters. For example: test <= 23 <= fest
2. Place the cursor after 23. Hit Ctrl+Backspace. As expected, 23 is deleted and the cursor stops at the old start of 23
3. Place the cursor before 23 and hit Ctrl+Delete. BUG: instead of deleting '23', the '<=' character group and the space after is deleted.
- or -
4. Place the cursor at the beginning of the text 'test <= 23 <= fest' Hit Ctrl+Delete, this deletes 'test <= '

What is the expected behavior?
Ctrl+Delete is expected to delete all characters on the right of the cursor, depending on the first character:
- if the character is a symbol, delete until symbols or EOL is found
- if the character isn't symbol, delete until a symbol or EOL is found

Note: Ctrl+Backspace works as expected.

What went wrong?
Ctrl+Delete deletes more characters as expected.

Did this work before? No 

Chrome version: 56.0.2924.87  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version:

 

Comment 1 by ajha@chromium.org, Mar 9 2017

Components: -UI UI>Input>KeyboardShortcuts
Labels: Needs-Triage-M56
Cc: kkaluri@chromium.org
Labels: -Type-Bug -Pri-2 -Needs-Triage-M56 hasbisect-per-revision M-59 Pri-1 Type-Bug-Regression
Owner: yosin@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on Windows 10 with stable #57.0.2987.98 and canary #59.0.3036.0
Issue broken in M52

Bisect Info:
===========

Good build : 52.0.2741.0,  Revision Range - 394609 	
Bad build  : 52.0.2742.0,  Revision Range - 394842	

After executing the per-revision-bisect script, i got the following CL's between good and bad build versions
===========================================
https://chromium.googlesource.com/chromium/src/+log/52328753422fc25a3815bd58f6eaf8e70f6e27ff..86d103eaec6f31e3c11d1ea7698231f4a589b871


The suspecting Change Log is :
-----------
https://chromium.googlesource.com/chromium/src/+/86d103eaec6f31e3c11d1ea7698231f4a589b871

Review-Url: https://codereview.chromium.org/1987973003

Note:
-----
Not seen this issue in Ubuntu 14.04 and Mac 10.12.3

yosin@- Could you please look into this issue, if it's related to your change?  if not could you please help us to reassign this issue to the right owner.

Comment 3 by yosin@chromium.org, Mar 10 2017

Components: -UI>Input>KeyboardShortcuts Blink>Editing>Selection
Status: Started (was: Assigned)
Summary: Selection#modify() with word granularity should not count punctuation as part of word (was: Editing: Ctrl+delete isn't working as Ctrl+backspace)
See https://jsfiddle.net/ugLp58px/

Comment 4 by yosin@chromium.org, Mar 15 2017

Labels: -Type-Bug-Regression Type-Feature
I don't think it works as expected so far.

This feature is introduced by [1] on April 23, 2013, maybe shipped on M28.

[1] ​https://bugs.webkit.org/show_bug.cgi?id=110487 Ctrl+Shift+Right in Windows should select the spacing after the word

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 16 2017

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

commit ce0eb87c0601121d065792c19c487a4580b4dd3e
Author: yosin <yosin@chromium.org>
Date: Thu Mar 16 23:20:32 2017

Utilize assert_selection() in editing/pasteboard/4242293-1.html

This patch makes "4242293-1.html" to use |assert_selection()| to visualize
test result and renames to "copy_ending_at_end_of_block.html" to provide a hint
what this test script does for improving code health.

This patch is a preparation of patch[1].

[1] http://crrev.com/2754543004: Make Selection#modify() with word granularity
not to skip punctuation

BUG= 699903 
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/2752593006
Cr-Commit-Position: refs/heads/master@{#457602}

[delete] https://crrev.com/e9ffec31f17a567b537b260f772d649a8f437154/third_party/WebKit/LayoutTests/editing/pasteboard/4242293-1.html
[add] https://crrev.com/ce0eb87c0601121d065792c19c487a4580b4dd3e/third_party/WebKit/LayoutTests/editing/pasteboard/copy_ending_at_end_of_block.html

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 21 2017

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

commit b32e43af4dea2249bb8aff7c06dc69e255882039
Author: yosin <yosin@chromium.org>
Date: Tue Mar 21 08:47:11 2017

Introduce skipWhitespace() to share VisibleSeleciton::appendTrailingWhitespace() functionality

This patch Introduces |skipWhitespace()| to share functionality of
VisibleSeleciton::appendTrailingWhitespace() in implementation of double-click
to select word and word granularity move and extend of |Selection#modify()|.

This patch is a preparation of the patch[1].

[1] http://crrev.com/2754543004: Make Selection#modify() with word granularity
not to skip punctuation

BUG= 699903 
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/2762083002
Cr-Commit-Position: refs/heads/master@{#458351}

[modify] https://crrev.com/b32e43af4dea2249bb8aff7c06dc69e255882039/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
[modify] https://crrev.com/b32e43af4dea2249bb8aff7c06dc69e255882039/third_party/WebKit/Source/core/editing/VisibleUnits.h

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 27 2017

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

commit 117a5ba5073a1c78d08d3be3210afc09af96158c
Author: yosin <yosin@chromium.org>
Date: Mon Mar 27 03:45:52 2017

Make Selection#modify() with word granularity not to skip punctuation

This patch makes |Selection#modify()| with word granularity to work as same
as mouse double-clicking to avoid skipping punctuation characters.

Example: Ctrl+ArrowRight or selection.modify('move', 'forward', 'word')
Before this patch: |ab := bc -> ab := |bc
After this patch:  |ab := bc -> ab |:= bc
Where "|" represent caret.

Note: this is Windows only behavior.

BUG= 699903 
TEST=attached test especially for "first-letter-word-boundary.html"

Review-Url: https://codereview.chromium.org/2754543004
Cr-Commit-Position: refs/heads/master@{#459700}

[modify] https://crrev.com/117a5ba5073a1c78d08d3be3210afc09af96158c/third_party/WebKit/LayoutTests/editing/selection/modify_extend/extend_by_word_002.html
[modify] https://crrev.com/117a5ba5073a1c78d08d3be3210afc09af96158c/third_party/WebKit/LayoutTests/editing/text-iterator/first-letter-word-boundary.html
[modify] https://crrev.com/117a5ba5073a1c78d08d3be3210afc09af96158c/third_party/WebKit/Source/core/editing/SelectionModifier.cpp

Comment 8 by yosin@chromium.org, Mar 29 2017

Status: Fixed (was: Started)
Labels: TE-Verified-M59 TE-Verified-59.0.3061.3
Verified this issue on Windows-10 using chrome latest Dev #59.0.3061.3 by following the steps provided in the original comment using test case provided in comment #3. Observed Ctrl+Backspace and Ctrl+Delete works as expected. Hence adding TE-Verified label. 
699903.mp4
1.5 MB View Download

Sign in to add a comment