FOCUS: *some* editing commands shouldn't interact with unfocused selection
Reported by
dchau...@etouch.net,
Apr 20 2017
|
||||||||||||
Issue descriptionChrome Version: 60.0.3076.0 (Official Build)54d7e3050af7ebb57aa7cd99cef14037db03f3bf-refs/heads/master@{#465838} 32/64-bit. OS: Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.11.6,10.12.1) What steps will reproduce the problem? 1. Launch chrome and navigate to chrome://md-settings page. 2. Keep focus on 'SIGN IN TO CHROME' button using 'Tab' key. 3. Now, press down arrow key from keyboard and observe. Unable to scroll the Settings page using down arrow key. Should be able to scroll the Settings page using down arrow key This is a regression issue, broken in M-60 series, below is manual regression range. Good build: 59.0.3071.0 Bad build: 60.0.3072.0 Kindly review the attached screen-cast for reference.
,
Apr 24 2017
,
Apr 24 2017
Maybe related to Issue 707106 ?
,
Apr 27 2017
,
Apr 27 2017
,
Apr 27 2017
,
Apr 28 2017
The root cause is search box eats ArrowUp/ArrowDown even if search box doesn't have focus. When you hit Ctrl+V when focus in "SIGN IN TO CHROME", search box is populated by clipboard text. When we shrink window size to make search box to search icon, ArrowUp/ArrowDown make page scroll. To fix this issue, we should change: Editor::SelectionForCommand(Event*) to check whether selection has focus or not. If selection doesn't have focus, we should return empty selection from Editor::SelectionForCommand)
,
Apr 28 2017
,
May 10 2017
,
May 12 2017
,
May 15 2017
I think we need to be more careful about the interaction between editing commands and focus & selection: Document.execCommand is allowed by spec to work regardless of focus And even for user-triggered commands, some of them are supposed to work regardless of the focus. Examples: - undo/redo - unselect - selectAll? So what's the general principle?
,
May 16 2017
Discussed with yosin@ offline. The conclusion is that each editing command may or may not run from a user-triggered event Hence the solution is to manually make each enabled function (in EditorCommands.cpp) handle focus separately, instead of adding a one-for-all switch in Editor::HandleKeyboardEvent. We need a dashboard for that.
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e91966df2e6a57cafb584bfa72e19bc2e9bb620 commit 2e91966df2e6a57cafb584bfa72e19bc2e9bb620 Author: xiaochengh <xiaochengh@chromium.org> Date: Wed May 17 02:06:07 2017 Make EnabledInEditableText return false for user-triggered command and unfocused selection Certain editing commands shouldn't be enabled when they are user-triggered and the selection doesn't have focus. This patch adds checking of FrameSelection::SelectionHasFocus() in EnabledInEditableText to fix the behavior of these commands: BackwardDelete DeleteBackward DeleteBackwardByDecomposingPreviousCharacter DeleteForward DeleteToBeginningOfLine DeleteToBeginningOfParagraph DeleteToEndOfLine DeleteToEndOfParagraph DeleteToMark DeleteWordBackward DeleteWordForward ForwardDelete IgnoreSpelling InsertBacktab InsertHTML InsertLineBreak InsertNewline InsertParagraph InsertTab InsertText MoveBackward MoveDown MoveForward MoveLeft MovePageDown MovePageUp MoveParagraphBackward MoveParagraphForward MoveRight MoveToBeginningOfDocument MoveToBeginningOfLine MoveToBeginningOfParagraph MoveToBeginningOfSentence MoveToEndOfDocument MoveToEndOfLine MoveToEndOfParagraph MoveToEndOfSentence MoveToLeftEndOfLine MoveToLeftEndOfLineAndModifySelection MoveToRightEndOfLine MoveToRightEndOfLineAndModifySelection MoveUp MoveWordBackward MoveWordForward MoveWordLeft MoveWordRight Yank YankAndSelect BUG= 713607 , 722925 TEST=editing/selection/arrow_key_with_unfocused_selection.html Review-Url: https://codereview.chromium.org/2886933002 Cr-Commit-Position: refs/heads/master@{#472284} [add] https://crrev.com/2e91966df2e6a57cafb584bfa72e19bc2e9bb620/third_party/WebKit/LayoutTests/editing/selection/arrow_key_with_unfocused_selection.html [modify] https://crrev.com/2e91966df2e6a57cafb584bfa72e19bc2e9bb620/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
,
May 17 2017
,
May 23 2017
Tested the issue using Chrome Dev# 60.0.3107.4 on Windows, Mac and Linux and found the issue to be Fixed. Hence adding TE-Verified labels. Adding screen cast for further reference. Thank You. |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by sureshkumari@chromium.org
, Apr 20 2017Owner: yosin@chromium.org
Status: Assigned (was: Unconfirmed)