New issue
Advanced search Search tips

Issue 678258 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

PointerEvent should set movementX & movementY as in MouseEvent

Project Member Reported by mustaq@chromium.org, Jan 4 2017

Issue description

MouseEvent.movementX/Y is defined in the PointerLock spec:
https://w3c.github.io/pointerlock/#extensions-to-the-mouseevent-interface

The PointerLock spec above defines the values even for unlocked pointers. Even though defining PointerLock behavior for PointerEvents could be tricky, movementX/Y for unlocked pointers should be straightforward. We need to fix this.
 
To clarify, we want this only for pointerType=mouse, at least for now.

Here is the spec discussion:
https://github.com/w3c/pointerevents/issues/131
Labels: M-57
Looks like this has been fixed in Edge for all pointerTypes (try http://rbyers.github.io/eventTest.html).  Can we do the same?
Status: Started (was: Available)
The change to fix it for mouse is fairly small. But to add it to other types of the events I need to add the fields to the touch events path from Chrome to Blink as we do this movement logic all in Chrome.
I believe the reason the movement X/Y calculation is outside Blink is due to the lock action. But since we are not going to have the lock action for touch pointers I can do this bookkeeping and calculation in Blink at the very end.
So here are the options.
1. Just a small fix for mouse nothing for non-mouse
2. small fix for mouse + a fix for non-mouse at the end in Blink.
3. small fix for mouse + bookkeeping outside of blink similar to mouse and all the plumbing.

Which one do you think is better?
Also do you think this is critical enough that we need to cherry pick this to the release branch as well? If so 3 wouldn't be an option anymore.
We will be fixing the mouse case anyway, so let's go for it and then possibly merge to M56.

For non-mouse pointers, if the WG reaches consensus on Rick's suggestion to support PointerLock for non-mouse pointers, we have to go for Option 3 I believe.
  https://github.com/w3c/pointerevents/issues/131#issuecomment-270403820
Otherwise Option 2 is better. So I suggest waiting for the spec discussion.

Rick, wdyt?

Comment 5 by rbyers@chromium.org, Jan 16 2017

Right we discussed on the call that we'd change to match Edge (handle non-mouse case similarly to mouse, so I guess that means we need to know the last known position for each pointerId?).
This is what I do in this CL for non-mouse.
https://codereview.chromium.org/2624783002/

For mouse ones I just use the one we already set in MouseEvents.
Basically I went with the second solution in comment #3.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 10 2017

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

commit 2fe7948da152f6d65725173f0ec1fa916e3a016c
Author: nzolghadr <nzolghadr@chromium.org>
Date: Fri Feb 10 20:14:16 2017

Fix movementX/Y attributes for touch pointer events

Keep track of previous coordinates of touch points and calculate the deltas
and store them inside the WebTouchPoints. Since pointerevents have
movementX/Y attributes which they expose to js this CL calculates the
correct values for movementX/Y of touch pointerevents.

BUG= 678258 

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

[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/content/browser/renderer_host/input/input_router_impl.h
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/content/browser/renderer_host/input/touch_input_browsertest.cc
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js
[add] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerlock/pointerevent_movementxy-manual-automation.js
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/Source/core/events/PointerEventFactory.cpp
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/Source/core/events/PointerEventFactoryTest.cpp
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/Source/platform/WebTouchEvent.cpp
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/public/platform/WebMouseEvent.h
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/third_party/WebKit/public/platform/WebPointerProperties.h
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/ui/events/blink/blink_event_util.cc
[modify] https://crrev.com/2fe7948da152f6d65725173f0ec1fa916e3a016c/ui/events/blink/blink_event_util_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment