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

Issue 766486 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Pointercancel not fired on kevin when using stylus

Project Member Reported by dtapu...@chromium.org, Sep 19 2017

Issue description

Use ToT
Load: http://jsbin.com/bogasefeyo/edit?html,js,output

Pointercancel is not dispatched when scrolling starts when using the stylus.
 
I tracked this down to an error in https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/input/PointerEventManager.cpp?q=BlockTouchPointers&sq=package:chromium&l=266

Seems we only look up pointer ids of type touch.

Navid, what do you think we should do here? I presume we should probably have an ifdef for chromeos/windows that also checks the stylus type as well?

Comment 2 by mustaq@chromium.org, Sep 25 2017

ifdefs are okay as a short term fix.  In any case, we should really focus on a longer-term fix: implement & ship InputDeviceCapabilities.pointerMovementScrolls, and use it as the condition here.

http://wicg.github.io/InputDeviceCapabilities/#dom-uievent-sourcecapabilities


Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4 2017

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

commit ca109349f4522f52aaf6b87e726ccf767a2f39d0
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Wed Oct 04 03:35:21 2017

Send pointercancel based on scroll capability of pointers

Store whether a pointer is capable of scrolling in
PointerEventManager (based on whether it is coming
from touch path or not). Then when scroll happens
send pointercancel for all scroll capable pointers
instead of only touch pointers.

Bug:  766486 
Change-Id: I7ae6808fe3c976fcb2dd3d123f22e870011476ab
Reviewed-on: https://chromium-review.googlesource.com/695841
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506292}
[modify] https://crrev.com/ca109349f4522f52aaf6b87e726ccf767a2f39d0/third_party/WebKit/Source/core/events/PointerEventFactory.cpp
[modify] https://crrev.com/ca109349f4522f52aaf6b87e726ccf767a2f39d0/third_party/WebKit/Source/core/events/PointerEventFactory.h
[modify] https://crrev.com/ca109349f4522f52aaf6b87e726ccf767a2f39d0/third_party/WebKit/Source/core/events/PointerEventFactoryTest.cpp
[modify] https://crrev.com/ca109349f4522f52aaf6b87e726ccf767a2f39d0/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Status: Fixed (was: Assigned)
Labels: Merge-Request-62
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 5 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
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), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: OS-Windows
abdulsyed@ bhthompson@ can you take a look at this? The change's been in the master for about 2 days and I also tested it with the Canary build that came out today.
Cc: abdulsyed@chromium.org bhthompson@chromium.org
Thanks for the fix - can you please help explain what the fix is, is it truly critical for M62, or can it wait until 63?
As far as I can tell it it critical for 62. Tom can elaborate on it more from Chrome OS side. But from Windows side we had some fixes regarding the drag/scrolling/selection behavior of pens on Windows and without this fix they are not complete.

Basically what this fix does it to correctly send javascript events when browser starts consuming the event stream say for scrolling by pen. Without this fix, a page would not have a way of finding about the fact that the event stream is cut by the browser.
If Abdul is good with it, I am good with it.
Labels: -Merge-Review-62 Merge-Approved-62
Great thanks for confirming approving merge to M62. Based on comment 10, approving merge to M62. Branch:3202
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 6 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a11800062c65e2e1d305ab18402c48a3833dd9eb

commit a11800062c65e2e1d305ab18402c48a3833dd9eb
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Fri Oct 06 17:03:44 2017

Send pointercancel based on scroll capability of pointers

Store whether a pointer is capable of scrolling in
PointerEventManager (based on whether it is coming
from touch path or not). Then when scroll happens
send pointercancel for all scroll capable pointers
instead of only touch pointers.

Bug:  766486 
Change-Id: I7ae6808fe3c976fcb2dd3d123f22e870011476ab
Reviewed-on: https://chromium-review.googlesource.com/695841
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#506292}(cherry picked from commit ca109349f4522f52aaf6b87e726ccf767a2f39d0)
Reviewed-on: https://chromium-review.googlesource.com/705314
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#607}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/a11800062c65e2e1d305ab18402c48a3833dd9eb/third_party/WebKit/Source/core/events/PointerEventFactory.cpp
[modify] https://crrev.com/a11800062c65e2e1d305ab18402c48a3833dd9eb/third_party/WebKit/Source/core/events/PointerEventFactory.h
[modify] https://crrev.com/a11800062c65e2e1d305ab18402c48a3833dd9eb/third_party/WebKit/Source/core/events/PointerEventFactoryTest.cpp
[modify] https://crrev.com/a11800062c65e2e1d305ab18402c48a3833dd9eb/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Sign in to add a comment