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

Issue 715889 link

Starred by 2 users

Issue metadata

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


Sign in to add a comment

Decoupling focus and selection causes regressions

Project Member Reported by yosin@chromium.org, Apr 27 2017

Issue description

This issue is a meta bug for regression caused by patch[1].
Since patch[1] improves compatibility among the browsers, 
and these regressions depend on buggy behavior fixed by patch[1].

We would like to try workaround of them or asking page owners to change not to depend
on buggy behavior.


As of today following issues report regression:
 -  Issue 712094  Regression:Add language navigates to the top after selecting language
 -  Issue 712986  Broken autoscroll when focusing hidden selection
 -  Issue 713051  Blink should not accept key stroke and blink caret when selection doesn't have focus
 -  Issue 713607  Regression: [MD Settings] Unable to scroll the Settings page using up/down arrow key
 -  Issue 714000  Regression: Unnecessary blue line appears on person name textbox on clicking any Avatar icon
 -  Issue 714525  Regression: Cursor pin remains stuck at 'Enter custom web address' field even after tap/touch on New Tab page
 -  Issue 714535  Regression:Auto-scroll of page after removing words in manage spell check of md-settings
 -  Issue 715036  Regression: Cursor is missing on Add Address overlay text fields.
 -  Issue 715038  Regression : Focus issue is seen on chrome://history/ page
 -  Issue 715059  Wrong context menu after hiding selection
 -  Issue 715353  MD Settings: Enter is ignored inside /networks?type=WiFi (ChromeOS)

[1] http://crrev.com/2616623002: Do not send redundant selectionchange-events (decouple focus)

 

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

Labels: M-60
Blockedon: -715353

Comment 3 by hu...@opera.com, May 2 2017

Blockedon: 717453

Comment 4 by hu...@opera.com, May 4 2017

Blockedon: 717955 718367
Blocking: 718395

Comment 6 by yosin@chromium.org, May 8 2017

Blockedon: 718800

Comment 7 by hu...@opera.com, May 8 2017

Blockedon: 718720
Blockedon: 718327

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

Once patch[1] is land, we can fix these bugs by checking whether selection has
focus or not by using FrameSelection::HasFocus().


[1] http://crrev.com/2841093002: Algorithm for deciding if a frame's selection should be hidden
Project Member

Comment 10 by bugdroid1@chromium.org, May 11 2017

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

commit 8549ed4eaf9d95db494bd7028bf17110b26b6918
Author: hugoh <hugoh@opera.com>
Date: Thu May 11 05:28:29 2017

Algorithm for deciding if a frame's selection should be hidden

Background:
Crrev.com/464698 introduced "hiding" of unfocused selections
in text controls. Hiding avoids clearing the selection upon change
of focus.

Problem:
Now we only hide selections inside text controls.
Selections within content-editable elements must also be hidden.

Solution:
Generalize previous work into an algorithm that, given current
DOM and its activeElement, determines whether a frame's selection
should be hidden.

See the algorithm in InHidden() for documentation and read its
unit tests in FrameSelectionTest.cpp.

BUG= 715059 ,  715889 

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

[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/LayoutSelection.h

Comment 11 by hu...@opera.com, May 16 2017

Blockedon: 722758
Project Member

Comment 12 by bugdroid1@chromium.org, May 16 2017

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

commit 582a4088398728aa9b1ab672dd92ecc9b9d589db
Author: hugoh <hugoh@opera.com>
Date: Tue May 16 16:35:59 2017

Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG= 715889 
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/css/SelectorChecker.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/layout/LayoutTextControlSingleLine.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/layout/LayoutTheme.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/core/paint/ImagePainter.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
[modify] https://crrev.com/582a4088398728aa9b1ab672dd92ecc9b9d589db/third_party/WebKit/Source/web/WebViewImpl.cpp

Blockedon: 722925

Comment 14 by yosin@chromium.org, May 19 2017

Status: Started (was: Available)
yosin@ is a virtual owner.

Comment 15 by yosin@chromium.org, May 19 2017

Blockedon: 714041

Comment 16 by yosin@chromium.org, May 19 2017

Owner: yosin@chromium.org

Comment 17 by yosin@chromium.org, May 23 2017

Blockedon: 724925

Comment 18 by yosin@chromium.org, May 23 2017

Blockedon: 725022

Comment 19 by yosin@chromium.org, May 23 2017

Blockedon: 725005

Comment 20 by yosin@chromium.org, May 23 2017

Blockedon: 536747

Comment 21 by yosin@chromium.org, May 24 2017

Status: Fixed (was: Started)
It seems all issues related to decoupling focus and selection are fixed.

Comment 22 by hu...@opera.com, Jun 22 2017

Blockedon: 735852

Comment 23 by hu...@opera.com, Jul 7 2017

Blockedon: 738830

Comment 24 by hu...@opera.com, Jul 20 2017

Blockedon: 740518
Components: Blink>HTML>Focus
Components: -Blink>Focus

Sign in to add a comment