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

Issue 714000 link

Starred by 5 users

Issue metadata

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

Blocking:
issue 715889



Sign in to add a comment

FOCUS: SelectionController::UpdateSelectionForMouseDrag() should ignore unfocused seleciton

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

Issue description

Chrome Version: 60.0.3077.0 (Official Build)ca8708632799fafe05e889d5a026249fa630dbab-refs/heads/master@{#466199} 32/64-bit.
OS: Windows(7,8,10), Linux(14.04 LTS), Mac(10.11.6, 10.12.1, 10.12).

What steps will reproduce the problem?
1. Launch chrome and navigate to chrome://settings page.
2. Click on profile icon under 'People' section to open 'Edit person' overlay.
3. Now, enter any long name in person name text box. 
4. Click on any avatar icon and observe the person name textbox.

On clicking avatar icon, Unnecessary blue line appears on person name textbox.
On clicking avatar icon, blue line should not appear on person name textbox.

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
1.5 MB View Download
Expected behavior.mp4
1.1 MB View Download

Comment 1 by dchau...@etouch.net, Apr 21 2017

Labels: -OS-Mac
Correction: This issue is not reproducible on Mac OS.
Cc: jmukthavaram@chromium.org hu...@opera.com
Labels: -OS-Linux hasbisect-per-revision ReleaseBlock-Stable
Owner: yosin@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on windows 7 with Chrome Canary.

Manual Bisect:
Good build: 59.0.3071.0-Revision-464641
Bad build: 60.0.3072.0 -Revision-464836

Per revision Bisect Tool Info:
You are probably looking for a change made after 464697 (known good), but no later than 464698 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspectas some perf builds might  get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/188c1056bbfb9c98eca691c12f5641d4204f6066..0f65d25a4097959d977dbb1077717323438240a6

Yosin@ assigning to you, as you were listed as one of the reviewers for this CL.

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 reproduce the issue on Linux Ubuntu 14.04 
Thanks.!

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

Blocking: 715889

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

Labels: -M-60

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

Components: Blink>Editing>Selection
Owner: ----
Status: Available (was: Assigned)
Summary: FOCUS Regression: Unnecessary blue line appears on person name textbox on clicking any Avatar icon. (was: Regression: Unnecessary blue line appears on person name textbox on clicking any Avatar icon. )

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

Labels: M-60

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

It seems "click" event of avatar moves focus to user name input box.

When you can drag within most left avatar icon, selection range is moved in input box.

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

We need to check whether selection is active or not in
SelectionController::HandleSingleClick()

For example:
202 if (extend_selection && !selection.IsNone()) {
203    // Note: "fast/events/shift-click-user-select-none.html" makes
204    // |pos.isNull()| true.
205    const PositionInFlatTree& pos = AdjustPositionRespectUserSelectAll(
206        inner_node, selection.Start(), selection.end(),

So, we should change

180  const VisibleSelectionInFlatTree& selection =
181      this->Selection().ComputeVisibleSelectionInFlatTree();
    
to
const VisibleSelectionInFlatTree& selection
  Selection().IsActive() ? Selection.ComputeVisibleSelectionInFlatTree()
                         : VisbileSelectionInFlatTree();


We may also need to change |Selection()| reference at L188:

185  if (FrameView* view = frame_->View()) {
186    const LayoutPoint v_point = view->RootFrameToContents(
187        FlooredIntPoint(event.Event().PositionInRootFrame()));
188    if (!extend_selection && this->Selection().Contains(v_point)) {
189      mouse_down_was_single_click_in_selection_ = true;
190      if (!event.Event().FromTouch())
191        return false;
192
193      if (!this->Selection().IsHandleVisible()) {
194        UpdateSelectionForMouseDownDispatchingSelectStart(
195            inner_node, selection, kCharacterGranularity,
196            HandleVisibility::kVisible);
197        return false;
198      }
199    }
200  }



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

Cc: dbeam@chromium.org dpa...@chromium.org

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

Components: -UI>Settings
Summary: SelectionController::HandleSingleClick() should check whether selection is active or not (was: FOCUS Regression: Unnecessary blue line appears on person name textbox on clicking any Avatar icon. )

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

Cc: sureshkumari@chromium.org
 Issue 715036  has been merged into this issue.
Labels: -ReleaseBlock-Stable
yosin@, do we have any latest updates on this issue? Also removing 'RB-Stable' label.

Thank you!

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

I'm waiting for patch[1] to land for Selection.IsActive(), it is now Selection.HasFocus() though.


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

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

Summary: FOCUS: SelectionController::HandleSingleClick() should check whether selection is active or not (was: SelectionController::HandleSingleClick() should check whether selection is active or not)

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

Owner: yosin@chromium.org
Status: Started (was: Available)

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

It seems dragging avatar causes dragging in INPUT element:

===== EventHandler::HandleMousePressEvent
____Document::SetFocusedElement 0x0000031a6fe534f0 <paper-button>
         {selection_behavior=kNone (2) type=kWebFocusTypeMouse (7) source_capabilities={...} }
DispatchBlur
DispatchFocusOut
DispatchDOMFocusOut
DispatchFocus
DispatchFocusIn
DispatchDOMFocusIn
_FC::SetFocusedElement Doc::SetFocusedElement = true
_FC::SetFocusedElement: END: element=0x0000031a6fe534f0 <paper-button>
        
MDM::HandleMouseFocus: kNotHandled
=MDM::HandleMousePressEvent
=== SC::HandleSingleClick: inner_node=0x0000031a6fe534f0 <paper-button>
        
=EH::HandleMousePress: event_result=kNotHandled (0)
================= EH::HandleMouseMoveEvent
FC::SetFocusedElement 0x0000031a6fe451d0 <input> 	blink_core.dll!blink::FocusController::SetFocusedElement
	blink_core.dll!blink::FrameSelection::SetFocusedNodeIfNeeded
	blink_core.dll!blink::FrameSelection::DidSetSelectionDeprecated
	blink_core.dll!blink::FrameSelection::SetSelection
	blink_core.dll!blink::FrameSelection::SetSelection
	blink_core.dll!blink::SelectionController::SetNonDirectionalSelectionIfNeeded
	blink_core.dll!blink::SelectionController::UpdateSelectionForMouseDrag
	blink_core.dll!blink::SelectionController::HandleMouseDraggedEvent
	blink_core.dll!blink::MouseEventManager::HandleMouseDraggedEvent
	blink_core.dll!blink::EventHandler::HandleMouseMoveOrLeaveEvent
	blink_core.dll!blink::EventHandler::HandleMouseMoveEvent
	blink_web.dll!blink::PageWidgetEventHandler::HandleMouseMove
	blink_web.dll!blink::PageWidgetDelegate::HandleInputEvent
 

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

Below is repro case. When we drag mouse in orange area,
selected range in INPUT element is changed to follow
mouse pointer.

<style>
.avatar {
  display: inline-block;
  border: 2px green solid;
  background-color: orange;
  margin: 10px;
  width: 50px;
  height: 50px;
  user-select: none;
}
</style>
<input value="0123456789">
<br>
<foo class=avatar></foo>
<foo class=avatar></foo>
<script>
const input = document.querySelector('input');
input.setSelectionRange(3, 6);
input.focus();
</script>

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

 Issue 722758  has been merged into this issue.

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

Summary: FOCUS: SelectionController::UpdateSelectionForMouseDrag() should ignore unfocused seleciton (was: FOCUS: SelectionController::HandleSingleClick() should check whether selection is active or not)

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

chrome://md-settings/manageProfile is easy way to access the problem page.

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

In review: http://crrev.com/2891693002: Make mouse drag to ignore unfocused selection

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

Cc: yosin@chromium.org
 Issue 718327  has been merged into this issue.
Project Member

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

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

commit d541129f1b272eb3e611b59b5e12dd0be0a4ba58
Author: yosin <yosin@chromium.org>
Date: Fri May 19 08:42:05 2017

Make mouse drag to ignore unfocused selection

This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to
ignore unfocused selection to make mouse dragging not to change unfocused
selection.

This patch updates "drag_user_select_none.html" to expect that there are
no selection when starting from "user-select:none".

BUG= 481985 , 714000 
TEST=run_layout_test editing/input/drag_in_unselectable.html

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

[add] https://crrev.com/d541129f1b272eb3e611b59b5e12dd0be0a4ba58/third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html
[modify] https://crrev.com/d541129f1b272eb3e611b59b5e12dd0be0a4ba58/third_party/WebKit/LayoutTests/editing/selection/mouse/drag_user_select_none.html
[modify] https://crrev.com/d541129f1b272eb3e611b59b5e12dd0be0a4ba58/third_party/WebKit/Source/core/editing/SelectionController.cpp

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

Status: Fixed (was: Started)
Retested this issue on Windows machine using build# 60.0.3107.4, it seems to be fixed and working as intended.

Attaching screen-cast for the same
60.0.3107.4 -behavior.mp4
1.2 MB View Download
Labels: TE-Verified-60.0.3107.4 TE-Verified-M60
As per Comment# 25, the issue is fixed on Latest Dev# 60.0.3107.4, hence adding TE-Verified Labels.
Thank You.
Hi,
I am afraid that the way this issue was fixed introduced a side effect.
Now in some cases it is impossible to select a link.
Seen on all platforms : copy following text to address bar
data:text/html,<title>Test Page</title><h1>Page with link</h1>The following text: <a href=%E2%80%9Chttp://www.google.com%E2%80%9D style=%E2%80%9Cdisplay: inline-block%E2%80%9D>click here</a> is a link
Hover the mouse over the "The following text..." line and move from left to right
When the cursor is over the first "c" of "Click here", left-click and drag to the right as to select link text
Selection is not possible.
This was broken by https://chromium.googlesource.com/chromium/src/+/d541129f1b272eb3e611b59b5e12dd0be0a4ba58

Comment 28 by hu...@opera.com, Aug 17 2017

yorenault@, <a> is not allowed to start a selection. I see the same behavior in Chrome 59 (before d541129f1) and in Firefox 54.0.1.

Sign in to add a comment