1773381 - In scroll frames which are scrollable both horizontally and vertically, scroll gestures are often ignored

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Markus Stange [:mstange]]

Steps to reproduce:

1. Be on macOS.
2. Go to https://share.firefox.dev/3H3IIvW
3. In the call tree, try to scroll down to the bottom right by making individual axis aligned scroll motions: First scroll straight down, then lift the fingers, then scroll straight to the right, lift the fingers again, then down again, etc.

Expected results:
Scrolling should work.

Actual results:
When the scroll gesture is in a different direction than the previous scroll gesture, it is often ignored.
Even worse, "ignored" scrolls to the left end up navigating to the previous page.

Comment 1

[Markus Stange [:mstange]]

I'm not sure if this is a regression.

Comment 2

[Dan Robertson (:dlrobertson)]

(In reply to Markus Stange [:mstange] from comment #1)

I'm not sure if this is a regression.

Have you reproduced this with a horizontal swipe while the horizontal scroller still has a significant amount of horizontal space in the given direction left to scroll? I can reproduce the blocked horizontal scroll with no effect, but I haven't been able to reproduce the blocked horizontal scroll that triggers a history swipe when there is horizontal space.

Comment 3

[Markus Stange [:mstange]]

Attachment: screen recording

Comment 4

[Markus Stange [:mstange]]

Yes, for me the history swipe occurs every time I have a blocked horizontal scroll in a direction where history swiping is possible (i.e. whenever the browser's back/forward button is enabled).

Comment 5

[Dan Robertson (:dlrobertson)]

(In reply to Markus Stange [:mstange] from comment #4)

Yes, for me the history swipe occurs every time I have a blocked horizontal scroll in a direction where history swiping is possible (i.e. whenever the browser's back/forward button is enabled).

Thanks for the video! That helped me understand the sequence of events and how the gestures were conducted. After attempting something similar I am able to reproduce as well.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1713403

Comment 7

[Dan Robertson (:dlrobertson)]

Summary

When implementing bug 1713403 I forgot to take into account Axis::CanScroll which is used by AsyncPanZoomController::ScrollableDirections and the APZInputBridge here.

Possible Fixes

ScrollableDirections should no longer consider axis locks

Should CanScroll consider the axis lock? Particularly when attempting to discover scrollable directions, this doesn't seem correct to me. A axis lock doesn't necessarily indicate the axis is not scrollable, it just means we're currently consuming velocity on that axis. Using the axis locks in CanScroll also kind of impacts our ability to break the axis lock when in STICKY mode (as seen here). I think it is possible that something close to this issue could be hit outside of dominant axis locking, but I haven't tested this yet.

Do not use axis locks to restrict the momentum scroll

Revert the change that extends axis locks past the pan gesture end into a possible momentum scroll (c7abfd8bc34c). Depending on the axis locking mode consume velocity on the weaker axis in the momentum scroll.

Comment 9

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1773381 - Axis lock should not be considered in CanScroll(). r=botond

The axis lock should not be considered when determining if an axis can
scroll in a given direction.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7afa986c3ce
Axis lock should not be considered in CanScroll(). r=botond

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b7afa986c3ce

Comment 12

[Pascal Chevrel:pascalc]

This is a regression that landed during the 102 nightly cycle, can we uplift it to our last betas? Thanks

Comment 13

[Dan Robertson (:dlrobertson)]

Comment on attachment 9280790 [details]
Bug 1773381 - Axis lock should not be considered in CanScroll(). r=botond

Beta/Release Uplift Approval Request

• User impact if declined: When the scroll gesture direction changes, the gesture may be ignored and possibly interpreted as a navigation gesture.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only impacts the flagging of a axis as scrollable if another gesture is locking the axis. The existing logic in AsyncPanZoomController::HandlePanningUpdate already handles this case.
• String changes made/needed:
• Is Android affected?: No

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9280790 [details]
Bug 1773381 - Axis lock should not be considered in CanScroll(). r=botond

Approved for 102 beta 8, thanks.

Comment 15

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/a02cd951f3af

Comment 16

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue on macOS 11.5 with Firefox 103.0a1 (2022-06-08) by following the steps from comment 0. The scroll stops or navigates to another page when changing the scroll direction.
The issue is verified fixed with Firefox 102.0b8 and Firefox 103.0a1 (2022-06-15) on macOS 11.5. The scroll works accordingly after following steps from comment 0.