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

Issue 735852 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 715889



Sign in to add a comment

I-beam is shown on mouse drag even when no text can be selected

Reported by rp...@etouch.net, Jun 22 2017

Issue description

Version: 61.0.3138.0 d2d3a3975e9c7f3c5c62ef0ecad2683332894600-refs/heads/master@{#481386}
OS: Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.11.6,10.12.3)

What steps will reproduce the problem?
1. Launch chrome, navigate to chrome://md-settings/manageProfile
2. Now drag mouse cursor in bottom area of page,observe mouse pointer 

Actual: Unnecessarily I beam pointer is seen while dragging mouse cursor when there is no text to select
Expected: Unnecessarily I beam pointer should not be seen while dragging mouse cursor when there is no text to select

This is regression issue, broken in ‘M 60’ and will soon update other info :
Good build:59.0.3071.0
Bad build: 60.0.3072.0
 
Actual_video.mp4
451 KB View Download
Expected_video.mp4
397 KB View Download
Labels: hasbisect-per-revision
Owner: hu...@opera.com
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 59.0.3071.0 (Revision: 464641).
Bad build : 60.0.3072.0 (Revision: 464836).

You are probably looking for a change made after 464697 (known good), but no later than 464698 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/188c1056bbfb9c98eca691c12f5641d4204f6066..0f65d25a4097959d977dbb1077717323438240a6

@hugoh: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.
Thank You.

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

Blocking: 715889
Cc: yosin@chromium.org hu...@opera.com
Owner: dpa...@chromium.org
dpapad@, do you recognize this ?

rpise@, if you write a small test case (a small JavaScript + HTML example) that triggers this bug - it is very much appreciated :)

Comment 3 by dpa...@chromium.org, Jun 23 2017

Labels: Needs-Feedback
@rpise: I am not able to reproduce this on Linux 61.0.3139.0.

Comment 4 by rp...@etouch.net, Jun 23 2017

Labels: -Needs-Feedback
With response to comment #3:
Rechecked the above issue on Window OS with latest canary chrome version :61.0.3139.0 and it is still reproducible.Kindly refer attached screen cast for reference.
Actual_video_61.0.3139.0_canary.mp4
223 KB View Download

Comment 5 by dpa...@chromium.org, Jun 27 2017

Components: -UI>Settings Blink>Editing>Selection
Owner: ----
Status: Available (was: Assigned)
@hugoh: This is happening everywhere, not just in settings, see minimal example at https://jsfiddle.net/nmc03ax4/2/. Un-assigning myself.

@rpise: I was able to reproduce after I understood what you meant by "drag the mouse". Clarifying for others here:

Click and hold the mouse button, then start moving the mouse.


Comment 6 by dpa...@chromium.org, Jun 27 2017

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

Comment 7 by yosin@chromium.org, Jun 27 2017

Summary: chrome://md-settings page should display arrow mouser cursor instead of i-beam cursor (was: Regression : Unnecessarily I beam pointer is seen in chrome://md-settings page.)
There are three places set I-beam cursor, all in EventHandler.cpp:

1. EventHandler::SelectCursor(); when hit test returns null-node
2. EventHandler::SelectCursor(); CSS property cursor:auto => SelectAutoCursor
3. EventHandler::SelectCursor(); CSS property cursor:text

We should check:
- ShouldShowIBeamForNode()
- EventHandler::SelectAutoCursor()

bool ShouldShowIBeamForNode(const Node* node, const HitTestResult& result) {
  if (!node)
    return false;

  bool layout_object_selectable = false;
  if (LayoutObject* layout_object = node->GetLayoutObject()) {
    PaintLayer* layer = layout_object->EnclosingLayer();
    if (layer->GetScrollableArea() &&
        layer->GetScrollableArea()->IsPointInResizeControl(
            result.RoundedPointInMainFrame(), kResizerForPointer)) {
      return false;
    }

    layout_object_selectable =
        layout_object->IsText() && node->CanStartSelection();
  }

  return HasEditableStyle(*node) || layout_object_selectable;
}

EventHandler::SelectAutoCursor()
  // During selection, use an I-beam no matter what we're over.
  // If a drag may be starting or we're capturing mouse events for a particular
  // node, don't treat this as a selection. Note calling
  // ComputeVisibleSelectionInDOMTreeDeprecated may update layout.
  if (mouse_event_manager_->MousePressed() &&
      GetSelectionController().MouseDownMayStartSelect() &&
      !mouse_event_manager_->MouseDownMayStartDrag() &&
      !frame_->Selection()
           .ComputeVisibleSelectionInDOMTreeDeprecated()
           .IsNone() &&
      !capturing_mouse_events_node_) {
    return i_beam;
  }

Comment 8 by yosin@chromium.org, Jun 27 2017

Owner: ----
Status: Available (was: Assigned)
I have started working on this and submitted a patch for review.. https://codereview.chromium.org/2967893002/

Status: Started (was: Available)
yosin@ is a virtual owner.
tanvir.rizvi@ is working as #c9.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 13 2017

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

commit 0fd45cda7fa9de64706b81ecc97354df7769eac3
Author: Hugo Holgersson <hugoh@opera.com>
Date: Thu Jul 13 07:47:23 2017

Show pointer (not I-beam) when mouse drags in the air

A bug surfaced when Blink started to allow hidden selections.
The condition "show I-beam if we have a non-empty selection"
became too generous.

We should show an I-beam when:
- Node is editable
- Node is selectable
- A link's text is being selected

Besides fixing the condition I moved its block into
ShouldShowIBeamForNode() to try to gather stray I-beam
logic into one place.

TEST=LinkSelectionTest.DragOnNothingShowsPointer

BUG= 735852 

Change-Id: I5366462c713155754de39b0c5d11fa33656eb6e3
Reviewed-on: https://chromium-review.googlesource.com/566798
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Hugo Holgersson <hugoh@opera.com>
Cr-Commit-Position: refs/heads/master@{#486308}
[modify] https://crrev.com/0fd45cda7fa9de64706b81ecc97354df7769eac3/third_party/WebKit/Source/core/editing/LinkSelectionTest.cpp
[modify] https://crrev.com/0fd45cda7fa9de64706b81ecc97354df7769eac3/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/0fd45cda7fa9de64706b81ecc97354df7769eac3/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/0fd45cda7fa9de64706b81ecc97354df7769eac3/third_party/WebKit/Source/core/input/EventHandlerTest.cpp

Comment 12 by hu...@opera.com, Jul 13 2017

Labels: -Pri-2 Merge-Request-60 Pri-1
Owner: hu...@opera.com
Status: Fixed (was: Started)
Summary: I-beam is shown on mouse drag even when no text can be selected (was: chrome://md-settings page should display arrow mouser cursor instead of i-beam cursor)
I'm requesting to merge this to Chrome 60. I know it is late in the game, but I think this is important to fix before it confuses end users.
Project Member

Comment 13 by sheriffbot@chromium.org, Jul 13 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Approved-60
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 18 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0e2b3e5681e73642b7ab69026ec888b0b22e104

commit e0e2b3e5681e73642b7ab69026ec888b0b22e104
Author: Hugo Holgersson <hugoh@opera.com>
Date: Tue Jul 18 07:00:49 2017

[Merge to M60] Show pointer (not I-beam) when mouse drags in the air

This is a manual merge of:

A bug surfaced when Blink started to allow hidden selections.
The condition "show I-beam if we have a non-empty selection"
became too generous.

We should show an I-beam when:
- Node is editable
- Node is selectable
- A link's text is being selected

Besides fixing the condition I moved its block into
ShouldShowIBeamForNode() to try to gather stray I-beam
logic into one place.

TEST=LinkSelectionTest.DragOnNothingShowsPointer

BUG= 735852 

(cherry picked from commit 0fd45cda7fa9de64706b81ecc97354df7769eac3)

Change-Id: I5366462c713155754de39b0c5d11fa33656eb6e3
Reviewed-on: https://chromium-review.googlesource.com/566798
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Hugo Holgersson <hugoh@opera.com>
Cr-Original-Commit-Position: refs/heads/master@{#486308}
Reviewed-on: https://chromium-review.googlesource.com/572904
Reviewed-by: Hugo Holgersson <hugoh@opera.com>
Cr-Commit-Position: refs/branch-heads/3112@{#629}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/e0e2b3e5681e73642b7ab69026ec888b0b22e104/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/e0e2b3e5681e73642b7ab69026ec888b0b22e104/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/e0e2b3e5681e73642b7ab69026ec888b0b22e104/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/e0e2b3e5681e73642b7ab69026ec888b0b22e104/third_party/WebKit/Source/web/tests/LinkSelectionTest.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 19 2017

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

commit 0edb193e1e3dd39f140353971e308023dc57a705
Author: Alex Mineer <amineer@chromium.org>
Date: Wed Jul 19 00:45:14 2017

Revert "[Merge to M60] Show pointer (not I-beam) when mouse drags in the air"

This reverts commit e0e2b3e5681e73642b7ab69026ec888b0b22e104.

Reason for revert: Breaking official Android builds

../../third_party/WebKit/Source/web/tests/LinkSelectionTest.cpp: In member function 'virtual void blink::LinkSelectionTest_DragOnNothingShowsPointer_Test::TestBody()':
../../third_party/WebKit/Source/web/tests/LinkSelectionTest.cpp:193:59: error: cannot convert 'base::TimeDelta' to 'double' for argument '1' to 'void blink::testing::RunDelayedTasks(double)'
   testing::RunDelayedTasks(TimeDelta::FromMilliseconds(50));

BUG= 735852 

Original change's description:
> [Merge to M60] Show pointer (not I-beam) when mouse drags in the air
> 
> This is a manual merge of:
> 
> A bug surfaced when Blink started to allow hidden selections.
> The condition "show I-beam if we have a non-empty selection"
> became too generous.
> 
> We should show an I-beam when:
> - Node is editable
> - Node is selectable
> - A link's text is being selected
> 
> Besides fixing the condition I moved its block into
> ShouldShowIBeamForNode() to try to gather stray I-beam
> logic into one place.
> 
> TEST=LinkSelectionTest.DragOnNothingShowsPointer
> 
> BUG= 735852 
> 
> (cherry picked from commit 0fd45cda7fa9de64706b81ecc97354df7769eac3)
> 
> Change-Id: I5366462c713155754de39b0c5d11fa33656eb6e3
> Reviewed-on: https://chromium-review.googlesource.com/566798
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Commit-Queue: Hugo Holgersson <hugoh@opera.com>
> Cr-Original-Commit-Position: refs/heads/master@{#486308}
> Reviewed-on: https://chromium-review.googlesource.com/572904
> Reviewed-by: Hugo Holgersson <hugoh@opera.com>
> Cr-Commit-Position: refs/branch-heads/3112@{#629}
> Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

TBR=yosin@chromium.org,bokan@chromium.org,hugoh@opera.com

Change-Id: Ic049226ef53be66fcbb2346a975a6457342c3bf2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  735852 
Reviewed-on: https://chromium-review.googlesource.com/576830
Reviewed-by: Alex Mineer <amineer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#648}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/0edb193e1e3dd39f140353971e308023dc57a705/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/0edb193e1e3dd39f140353971e308023dc57a705/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/0edb193e1e3dd39f140353971e308023dc57a705/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/0edb193e1e3dd39f140353971e308023dc57a705/third_party/WebKit/Source/web/tests/LinkSelectionTest.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 19 2017

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

commit e2f05e7c6a18b6bd6673e593fb97a70817a60d68
Author: Hugo Holgersson <hugoh@opera.com>
Date: Wed Jul 19 08:48:42 2017

Reland: [Merge to M60] Show pointer (not I-beam) when mouse drags in the air

TBR=bokan@chromium.org

Now, unit test is backported properly. This is a new manual merge of:

A bug surfaced when Blink started to allow hidden selections.
The condition "show I-beam if we have a non-empty selection"
became too generous.

We should show an I-beam when:
- Node is editable
- Node is selectable
- A link's text is being selected

Besides fixing the condition I moved its block into
ShouldShowIBeamForNode() to try to gather stray I-beam
logic into one place.

TEST=LinkSelectionTest.DragOnNothingShowsPointer

BUG= 735852 

Reviewed-on: https://chromium-review.googlesource.com/566798
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Hugo Holgersson <hugoh@opera.com>
Cr-Original-Original-Commit-Position: refs/heads/master@{#486308}
Change-Id: I9895ccbd4d8c3d506c5b9064e3d7cf5488c56914
Reviewed-on: https://chromium-review.googlesource.com/575063
Reviewed-by: Hugo Holgersson <hugoh@opera.com>
Cr-Commit-Position: refs/branch-heads/3112@{#650}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/e2f05e7c6a18b6bd6673e593fb97a70817a60d68/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/e2f05e7c6a18b6bd6673e593fb97a70817a60d68/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/e2f05e7c6a18b6bd6673e593fb97a70817a60d68/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/e2f05e7c6a18b6bd6673e593fb97a70817a60d68/third_party/WebKit/Source/web/tests/LinkSelectionTest.cpp

Sign in to add a comment