1783936 - CSS scroll-snap-type mandatory isn't triggered on touchscreen device correctly

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect)

Description

[olisevich]

Attachment: scroll-snap.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36

Steps to reproduce:

1. Follow to the MDN page about scroll-snap here https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-snap-type
2. Try the second example with 'scroll-snap-type: x mandatory' using touchscreen (both laptop and mobile)

The device used to repeat the issue were Microsoft Laptop 3 and Google Pixel 2.
The laptop OS = Windows 10;
The Android version = 11

Actual results:

The scroll is not snapped to the central point when using touchscreen input neither in desktop FF or android FF.
In order the snap to trigger the swipe should be very 'rough' or have 'momentum'.

Scrolling via scrollbar, trackpad or mouse working ad expected.

Expected results:

The same example in the Chrome browser triggers snap point correctly. No need to control the 'swipe' gesture on touchscreen.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Scrolling and Overflow' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[David Shin[:dshin]]

Reproducible with Ubuntu 20.04/105.0a1/laptop touchpads

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Regressed by bug 1766805, specifically this ScrollSnap call in SnapBackIfOverscrolled. We shouldn't specify ScrollSnapFlags::IntendedDirection since in the case of mState != FLING there's no intentional direction of the scrolling at that moment, it just means the user lifted their fingers without any further momentum scrolling.

Comment 4

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

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

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1783936 - Drop "ask the main thread to" words from a coment about scroll snap in AsyncPanZoomController::SnapBackIfOverscrolled. r?botond

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1783936 - Do scroll snap without IntendedDirection in the case of non-fling scrolling. r?botond

It matches "a panning gesture, released without momentum" case in "the type of
scrolling methods" [1] section in the spec.

[1] https://drafts.csswg.org/css-scroll-snap/#scroll-types

Depends on D155048

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8259db3325df
Drop "ask the main thread to" words from a coment about scroll snap in AsyncPanZoomController::SnapBackIfOverscrolled. r=botond
https://hg.mozilla.org/integration/autoland/rev/f6a94fda7fc3
Do scroll snap without IntendedDirection in the case of non-fling scrolling. r=botond

Comment 8

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

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

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/8259db3325df
https://hg.mozilla.org/mozilla-central/rev/f6a94fda7fc3

Comment 10

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

The patch landed in nightly and beta is affected.
:hiro, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.Also, don't forget to request an uplift for the patches in the regression caused by this fix.
• If no, please set status-firefox105 to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Okay, it's still at very early stage of the 105beta. I'd like to uplift because people might be more sensitive about scroll snap in fact it was in the relnote for 104 release.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9290595 [details]
Bug 1783936 - Do scroll snap without IntendedDirection in the case of non-fling scrolling. r?botond

Beta/Release Uplift Approval Request

• User impact if declined: Uses will feel some weird scroll interactions
• 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): The change is pretty simple, it should never cause any severe issues such as crash.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9290594 [details]
Bug 1783936 - Drop "ask the main thread to" words from a coment about scroll snap in AsyncPanZoomController::SnapBackIfOverscrolled. r?botond

Approved for 105.0b4.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5a7b0c3a383f
https://hg.mozilla.org/releases/mozilla-beta/rev/fb0e592b14a8