[touchpad] Pinch zooming on Google Maps moves the fixed position elements
Categories
(Core :: Panning and Zooming, defect, P3)
Tracking
()
People
(Reporter: gpalko, Assigned: tnikkel)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Affected versions
Nightly 79.0a1
Affected platforms
Devices with precision touchpad (tested on Lenovo yoga)
Preconditions
apz.allow_zooming = true
apz.windows.use_direct_manipulation = true
Steps to reproduce
- Open https://maps.google.com
- On map- Initiate pinch zooming from the touchpad
Expected result
The map is zoomed, other elements on the page remain displayed(buttons, search field)
Actual Result
The buttons from the page are moved as zooming is performed
The user needs to zoom out, in order to use the search box from the page or the control buttons on the bottom right side
Zoom out performance is poor
Note
Pinch zoom initiated from the touchscreen is working as expected
The issue looks similar to bug 1619187
Assignee | ||
Comment 1•4 years ago
|
||
Seems like for every pinch gesture (pinch gesture being user touches touch pad, moves fingers then lifts fingers) one event that should be preventdefault-ed by content is not and zooms the page, but the rest of the pinch gesture inputs work correctly.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Seems like the problem might be that we get a InputQueue::ReceivePinchGestureInput call before we get InputQueue::SetConfirmedTargetApzc. At least that is a difference on mac (where it works) vs Windows (where it doesn't) for me.
Assignee | ||
Comment 3•4 years ago
|
||
The problem is that this code
prevents us from dispatching the wheel event that we create from the first PinchGestureInput. When we send a pinch start or end from direct manipulation we send mPreviousSpan == mCurrent span here
and that creates a wheel event with all zero deltas. On mac we always seem to get a positive magnification even if we are sending a pinch start so we avoid this problem. And so on Windows we never dispatch because of this if
Assignee | ||
Comment 4•4 years ago
|
||
Looks like sending mPreviousSpan == mCurrentSpan is used in several other places. Should we be avoiding doing that in general? Or find a different way to solve this?
Assignee | ||
Comment 5•4 years ago
|
||
In the mean time I'll change direct manipulation to be like mac so we can fix this.
Assignee | ||
Comment 6•4 years ago
|
||
Updated•4 years ago
|
Comment 7•4 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #4)
Looks like sending mPreviousSpan == mCurrentSpan is used in several other places. Should we be avoiding doing that in general? Or find a different way to solve this?
I think the choice to send mPreviousSpan == mCurrentSpan pre-dates any use of wheel events generated based on the pinch gestures so this consideration hasn't come up before.
APZ itself only seems to pay attention to the spans for PINCHGESTURE_SCALE
events, so it should be safe to tweak places that send PINCHGESTURE_START
events to make sure they have a nonzero span change and thus get dispatched to content. (The alternative would be to check for generated-from-pinch events specifically in IsAllowedToDispatchDOMEvent()
, but I don't know if there's enough info on the wheel event to determine that, and it's probably not worth plumbing an extra field in.)
Comment 8•4 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #7)
(The alternative would be to check for generated-from-pinch events specifically in
IsAllowedToDispatchDOMEvent()
, but I don't know if there's enough info on the wheel event to determine that, and it's probably not worth plumbing an extra field in.)
There is a pre-existing mInputSource
field on WidgetMouseEventBase
(which is a superclass of WidgetWheelEvent
) that we use for similar things, but I'm not sure it's a good idea to actually do this. Presumably web content isn't expecting to handle zero-delta wheel events and if we start sending those it might cause unexpected side-effects.
I think for now making PINCHGESTURE_START
contain a nonzero span change seems like the right thing to do. It seems like this is going to be a trackpad-specific thing, because we only generate wheel events from trackpad pinches. So it's a fairly limited-scope problem, and we can revisit this later if we discover other cases where the solution could be unified with this one.
Updated•4 years ago
|
Updated•4 years ago
|
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b21421230463 Send pinch starts events with some amount of scale instead of none with direct manipulation. r=kats
Comment 10•4 years ago
|
||
bugherder |
Assignee | ||
Comment 13•4 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
I think for now making
PINCHGESTURE_START
contain a nonzero span change seems like the right thing to do. It seems like this is going to be a trackpad-specific thing, because we only generate wheel events from trackpad pinches. So it's a fairly limited-scope problem, and we can revisit this later if we discover other cases where the solution could be unified with this one.
So this means that we don't actually need a followup as the patch here has fixed all the cases that matter.
Description
•