1701077 - The overscroll animation's starting velocity can be diagonal when only one axis allows overscrolling

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[Markus Stange [:mstange]]

Steps to reproduce:

1. Be on macOS and have apz.overscroll.enabled set to true.
2. Be on a page that's only scrollable vertically.
3. Option A: Scroll up into overscroll and then fling to the side.
4. Option B: With a bit of distance from the top, do a slightly diagonal fling towards the top left.

Expected results:
Since we currently don't allow horizontal overscroll unless the page is scrollable horizontally (bug 1686151), the overscroll animation should only go upwards, not to the side.

Actual results:
The overscroll animation shifts the page horizontally and not just vertically.

This only happens for flings, i.e. if the overscroll is started with some velocity. I think we're respecting the horizontal component of the scrolling velocity vector when we should be ignoring it.
I'm hitting this all the time, for example I've hit it again in this textbox while typing this bug report. Most of my vertical flings have an unintended diagonal component to them.

Comment 1

[Markus Stange [:mstange]]

Something similar happens on both-axes-scrollable pages for slightly diagonal flings which are axis-locked to vertical flings. We scroll only vertically due to the axis lock, but then once we hit the top edge we forget about the axis lock and start a diagonal overscroll. (Edit: I filed bug 1701699 about this problem.)

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Looks like we should just call AdjustDeltaForAllowedScrollDirections for the velocity in StartOverscrollAnimation.

I tried it with GetCurrentPanGestureBlock()->GetAllowedScrollDirections() but it didn't work as expected since the GetAllowedScrollDirections() returns the result of AsyncPanZoomController::GetAllowedHandoffDirections.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1701077 - Factor out GetOverscrollableDirections. r?botond

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1701077 - Drop the overscroll animation's velocity on axes where we don't allow overscrolling. r?botond

Depends on D109835

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1701077 - Use ScrollDirections for OverscrollEffectBase::ConsumeOverscroll. r?botond

Depends on D109836

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77d0d14fe116
Factor out GetOverscrollableDirections. r=botond
https://hg.mozilla.org/integration/autoland/rev/bfdd43326ef9
Drop the overscroll animation's velocity on axes where we don't allow overscrolling. r=botond
https://hg.mozilla.org/integration/autoland/rev/16bc0a4dbf29
Use ScrollDirections for OverscrollEffectBase::ConsumeOverscroll. r=botond

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/77d0d14fe116
https://hg.mozilla.org/mozilla-central/rev/bfdd43326ef9
https://hg.mozilla.org/mozilla-central/rev/16bc0a4dbf29

Comment 8

[Andrei Purice]

Verified fixed in beta 89.0b2 (20210420191345) and Nightly 90.0a1 (2021-04-20) (20210420213949) on MacOS 11.2.3.