1848917 - Scroll-snap on screen-height blocks results in slow, broken, non-snapped scrolling

Status: RESOLVED FIXED

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

Description

[Nick Overacker [:Nick]]

Attachment: A performance graph of one of the slow transitions (not from the screen recording). Nothing remarkable stands out to me.

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/116.0

Steps to reproduce:

Repeatedly scroll forward and backward through the scroll snap slides at either the Codepen or JSFiddle below.

https://jsfiddle.net/kn7vgLht/
https://codepen.io/argyleink/pen/jOzGJej

Actual results:

After a few transitions, intermittent freezing mid-transition occurs. It then requires several scroll actions to proceed, far more than would be needed even with unsnapped blocks.

Screen-recorded demonstration: https://youtu.be/LzncbRONy5E

NOTE: Similar but not identical janky behavior occurs on Firefox for Android as well, so this problem may not be due to PC-specific code.

Expected results:

It should have transitioned fully and smoothly between every pair of slides with only one scroll event.

Comment 1

[Nick Overacker [:Nick]]

Note: I have also experienced this same bug in Windows on this same PC, so it is not a Linux-specific issue.

More details than can be fit into the format of a bug report can be found on my SO post.

Comment 2

[Nick Overacker [:Nick]]

CORRECTION:

The JSFiddle used incorrect CSS. Here is the corrected JSFiddle.

The Codepen was correct. The bug is still present in both after correction.

Comment 3

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

The severity field is not set for this bug.
:hiro, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8c6f1126927ead909b413c26f63e2945c221cd3e&tochange=a948931a259554f0f85c79db8cbf6f5ac2beb08f

Probably regressed by bug 1780701.

Comment 5

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

The severity field is not set for this bug.
:hiro, could you have a look please?

For more information, please visit BugBot documentation.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

The issue happens when we received a new wheel event while a Smooth(Msd) animation triggered by scroll-snap is running. So what happens exactly is;

1. A wheel event happens
2. The event triggers a Smooth(Msd) animation here and its destination is 600, for example
3. A new wheel event happens during the animation is running
4. Since bug 1780701, we don't return valid snap position from this MaybeAdjustDeltaForScrollSnappingOnWheelInput call because the destination (600) is same
5. Then unfortunately due to the Smooth(Msd) animation, mState is not WHEEL_SCROLL, thus we fall into this if block use initialPosition which is not factored into the previous animation state

So probably we need to tell whether there's the previous smooth animation triggered by the previous snap, and if there is, we need to bail out.

Comment 7

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

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

Comment 8

[Dianna Smith [:diannaS]]

:hiro any plan on working on this or should we stop tracking it?

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Yeah we should stop tracking, it's not a recent regression and I don't have cycles to work on this soon.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

I think this bug has been fixed by bug 1922904.

Comment 11

[Nick Overacker [:Nick]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #10)

I think this bug has been fixed by bug 1922904.

Bless and thank you!

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(I forgot closing)