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

Issue 689158 link

Starred by 5 users
Status: Assigned
Owner:
nzolghadr@chromium.org
Cc:
█ hdodda@chromium.org
█ o...@pettay.fi
rbyers@chromium.org
mustaq@chromium.org
dpa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 869919



Sign in to add a comment

Click event target while a pointer capture is in place is incorrect

Project Member Reported by nzolghadr@chromium.org, Feb 6 2017 
Steps to reproduce:
1. Navigate to this page: https://output.jsbin.com/zuwiwep
2. mouse press on blue div and release on green.
3. observe the click event target

4. mouse press on green div and release on blue.
5. observe the click event target


Expected:

3. The click target should be blue as both pointerdown and up are received by blue
5. The click target should be grey (the first common parent of blue and green) as pointerdown is received by green and pointerup is received by blue.

Comment 1 by michael....@gmail.com, Feb 8 2017 

We have encountered a similar issue when upgrading from Chrome 55 to 56. What is the version that you encountered this on?

Comment 2 by nzolghadr@chromium.org, Feb 8 2017 

I was testing on the tip of the tree. The issue should exists in Canary 58 as well. But if I recall correctly the issue should have always been there. That is weird that you were not seeing the issue on 55 unless we are not talking about the same issue.
Btw, are you using (developing) using pointerevent capture APIs? As this issue only happens when you use that.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 28 2017 

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

commit 14cea387ce9359e5f9fe3e90c996adf75e092e64
Author: nzolghadr <nzolghadr@chromium.org>
Date: Tue Feb 28 15:22:30 2017

Send click event after pointer capturing retarget

When calculating click event target we need to
use the captured target. Moving sending click
event further down in PointerEventManager allows
us to do that.

BUG=689158

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

[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_click_during_capture-manual-automation.js
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/editing/SelectionController.h
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/MouseEventManager.h
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/PointerEventManager.cpp
[modify] https://crrev.com/14cea387ce9359e5f9fe3e90c996adf75e092e64/third_party/WebKit/Source/core/input/PointerEventManager.h

Comment 4 by nzolghadr@chromium.org, Feb 28 2017

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 22 2017 

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

commit 44b8e6b6355c3a0fe61bc6bfb38fb63632618a81
Author: nzolghadr <nzolghadr@chromium.org>
Date: Wed Mar 22 14:29:53 2017

Revert "Send click event after pointer capturing retarget"

Reason for revert:
Reverting all the changes related to the click target in capturing as they are splitted across multiple commits.

This reverts commit 14cea387ce9359e5f9fe3e90c996adf75e092e64.

BUG=689158

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org

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

[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/72aaf8e63b4ead3f868748fb5a37c1cedec9caae/third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_click_during_capture-manual-automation.js
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/editing/SelectionController.h
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/MouseEventManager.h
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/PointerEventManager.cpp
[modify] https://crrev.com/44b8e6b6355c3a0fe61bc6bfb38fb63632618a81/third_party/WebKit/Source/core/input/PointerEventManager.h
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 27 2017

Labels: merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a1d897d7bea85eb9b073fce89743a311f7d46dc

commit 3a1d897d7bea85eb9b073fce89743a311f7d46dc
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Mon Mar 27 17:59:03 2017

Revert "Send click event after pointer capturing retarget"

Reason for revert:
Reverting all the changes related to the click target in capturing as they are splitted across multiple commits.

This reverts commit 14cea387ce9359e5f9fe3e90c996adf75e092e64.

BUG=689158

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org

Review-Url: https://codereview.chromium.org/2771463002
Cr-Commit-Position: refs/heads/master@{#458743}
(cherry picked from commit 44b8e6b6355c3a0fe61bc6bfb38fb63632618a81)
(cherry picked from commit 47ec688a15c4a44f86bbd02785d7e41accf6a986)

Review-Url: https://codereview.chromium.org/2781623002 .
Cr-Commit-Position: refs/branch-heads/3029@{#431}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/3b957ce7a6c9b3828c257c0193c1405a619fe75f/third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_click_during_capture-manual-automation.js
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/editing/SelectionController.h
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/EventHandler.h
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/MouseEventManager.h
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/PointerEventManager.cpp
[modify] https://crrev.com/3a1d897d7bea85eb9b073fce89743a311f7d46dc/third_party/WebKit/Source/core/input/PointerEventManager.h

Comment 7 by nzolghadr@chromium.org, Mar 27 2017

Status: Assigned (was: Fixed)
Reverted due to the regressions like  issue 699933  and  issue 701599 .

Comment 8 by hdodda@chromium.org, Mar 29 2017

Cc: hdodda@chromium.org
Labels: Needs-Feedback
Tested on windows 7 , mac os 10.12.3and ubuntu 14.04 using chrome M58 #58.0.3029.41 and followed the steps given in comment #0.

Observed following click events generated :

blue pointerdown
Set pointer capture to blue div
blue gotpointercapture
blue pointerup
blue lostpointercapture
grey click
green pointerdown
Set pointer capture to blue div
blue gotpointercapture
blue pointerup
blue lostpointercapture
grey click

Attached screenshot for the same.

@nzolghadr-- Could you please look into the screenshot and confirm us if this is the expected result and please confirm us if we had missed any steps in verifying the issue.

Thanks!
689158.png
167 KB View Download

Comment 9 by nzolghadr@chromium.org, Mar 29 2017 

Note that I reverted this change from the branch. So it is expected output in the sense that it was the incorrect behavior that existed before I tried to fix this bug. After I put the fix it caused regressions in M58 branch and I had to revert it. So I don't think you can mark this as verified or anything like that. But what you see there is what is expected to now have any regressions in the branch at the moment.

Comment 10 by dpa...@chromium.org, Dec 6 2017

Cc: dpa...@chromium.org

Comment 11 by nzolghadr@chromium.org, Oct 2

Blocking: 869919

Comment 12 by nzolghadr@chromium.org, Oct 25

Cc: o...@pettay.fi

Comment 13 by nzolghadr@chromium.org, Jan 15

Labels: -Pri-3 Pri-2
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 18 (5 days ago) 

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

commit b84997347f793ee460182e1d89907c812a956f53
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Fri Jan 18 17:24:29 2019

Consider capture target when sending click events

Captured targets should be used when calculating
the click target.

Bug: 689158
Change-Id: I733d0bfc8db6064cab4c1dd22e6a1b3b4ebb1598
Reviewed-on: https://chromium-review.googlesource.com/c/1407506
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ella Ge <eirage@chromium.org>
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624176}
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/event_handler.cc
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/event_handler.h
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/mouse_event_manager.cc
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/mouse_event_manager.h
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/pointer_event_manager.cc
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/renderer/core/input/pointer_event_manager.h
[modify] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/web_tests/NeverFixTests
[rename] https://crrev.com/b84997347f793ee460182e1d89907c812a956f53/third_party/blink/web_tests/external/wpt/pointerevents/pointerevent_click_during_capture.html

Comment 15 by dpa...@chromium.org, Jan 19 (5 days ago) 

@nzolghadr: Should I try removing the work-around at [1], or are there more CLs needed before the issue is fixed?


[1] https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_toggle/cr_toggle.html?l=95-100

Comment 16 by nzolghadr@google.com, Jan 20 (3 days ago) 

This change alone was breaking these toggles in Chrome settings. I needed the change in issue 869919 as well to get that fixed. Now both of them are landed and hopefully everything is still working with those toggles in the settings. This was the case as far as I tested but can you also confirm with the latest Canary or ToT?

Regarding removing your work-around, I would say wait until I close this issue. I have to make sure there isn't going to be more regressions and if there was something that we/they couldn't get the fix in I might need to revert these changes.
►

Sign in to add a comment