New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 713607 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 715889



Sign in to add a comment

FOCUS: *some* editing commands shouldn't interact with unfocused selection

Reported by dchau...@etouch.net, Apr 20 2017

Issue description

Chrome 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.
 
Actual behavior.mp4
899 KB View Download
Expected behavior.mp4
381 KB View Download
Labels: hasbisect-per-revision Proj-MaterialDesign-WebUI
Owner: yosin@chromium.org
Status: Assigned (was: Unconfirmed)
Manual Bisect info:
====================
Good build: 59.0.3071.0-(Revision-464641)
Bad build: 60.0.3072.0-(Revision-464836)

Per revision bisect Tool Info:
------------------------------
https://chromium.googlesource.com/chromium/src/+log/188c1056bbfb9c98eca691c12f5641d4204f6066..0f65d25a4097959d977dbb1077717323438240a6

Review-Url: https://codereview.chromium.org/2616623002
Yosin@ Kindly take a look and please help us to reassign this issue to a right owner if not with respect to this change.
Note:Unable to assign to owner of this CL, so assigning to reviewer.
Thanks.!
Labels: Hotlist-MD-Settings-PageA11y
Maybe related to  Issue 707106  ?

Comment 4 by yosin@chromium.org, Apr 27 2017

Blocking: 715889

Comment 5 by yosin@chromium.org, Apr 27 2017

Labels: -M-60

Comment 6 by yosin@chromium.org, Apr 27 2017

Components: Blink>Editing>Selection
Labels: M-60
Owner: ----
Status: Available (was: Assigned)
Summary: FOCUS Regression: [MD Settings] Unable to scroll the Settings page using up/down arrow key. (was: Regression: [MD Settings] Unable to scroll the Settings page using up/down arrow key.)

Comment 7 by yosin@chromium.org, Apr 28 2017

Cc: hu...@opera.com
Components: -UI>Settings
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)




Comment 8 by yosin@chromium.org, Apr 28 2017

Summary: Editor::SelectionForCommand() should check whether selection is active or not (was: FOCUS Regression: [MD Settings] Unable to scroll the Settings page using up/down arrow key.)

Comment 9 by yosin@chromium.org, May 10 2017

Summary: FOCUS: Editor::SelectionForCommand() should check whether selection is active or not (was: Editor::SelectionForCommand() should check whether selection is active or not)
Owner: xiaoche...@chromium.org
Status: Started (was: Available)
Summary: FOCUS: *some* editing commands shouldn't interact with unfocused selection (was: FOCUS: Editor::SelectionForCommand() should check whether selection is active or not)
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?
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.
Project Member

Comment 13 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Labels: TE-Verified-M60 TE-Verified-60.0.3107.4
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.
713607.ogv
2.7 MB View Download

Sign in to add a comment