1916137 - `scroll-behavior: smooth` should not be able to trigger an overscroll animation

Status: RESOLVED FIXED

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

Description

[Botond Ballo [:botond]]

In bug 1915006, I came across a testcase where a programmtic scroll triggered using scroll-behavior: smooth can cause a scroll frame to enter overscroll, and discovered that we have a codepath for this.

As discussed in bug 1915006 comment 7, I don't think this codepath should exist. This bug tracks removing it.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

https://searchfox.org/mozilla-central/rev/5f641898e12dafdfb7d54a93f4f3164d2c1642b2/gfx/layers/apz/src/SmoothMsdScrollAnimation.cpp#79-83

// The smooth scroll may have caused us to reach the end of our scroll
// range. This can happen if either the
// layout.css.scroll-behavior.damping-ratio preference is set to less than 1
// (underdamped) or if a smooth scroll inherits velocity from a fling
// gesture.

Either scenario is gated by a pref, layout.css.scroll-behavior.damping-ratio or apz.allow_immediate_handoff, it will not happen with default values. Setting P3:S3 for this reason.

Comment 2

[Botond Ballo [:botond]]

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

https://searchfox.org/mozilla-central/rev/5f641898e12dafdfb7d54a93f4f3164d2c1642b2/gfx/layers/apz/src/SmoothMsdScrollAnimation.cpp#79-83

// The smooth scroll may have caused us to reach the end of our scroll
// range. This can happen if either the
// layout.css.scroll-behavior.damping-ratio preference is set to less than 1
// (underdamped) or if a smooth scroll inherits velocity from a fling
// gesture.

Either scenario is gated by a pref, layout.css.scroll-behavior.damping-ratio or apz.allow_immediate_handoff, it will not happen with default values. Setting P3:S3 for this reason.

That may be what the comment claims, but in the test case of bug 1915006 we did observe overscroll happening without either pref being modified.

Comment 3

[Botond Ballo [:botond]]

Attachment: Bug 1916137 - Do not allow SmoothMsdScrollAnimation to trigger scroll handoff or overscroll. r=hiro

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)

in the test case of bug 1915006 we did observe overscroll happening without either pref being modified

The way I'm seeing it happen in the test case is that APZ gets a ScrollPositionUpdate with a very large destination (x=70000 or such).

The destination is well out of range, so it seems suspicious that this happens in the first place. I filed bug 1918834 to investigate further.

However, from APZ's point of view, it should avoid getting into overscroll even if such a thing happens, so I posted a patch + gtest here to do that.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b0037f7b822
Do not allow SmoothMsdScrollAnimation to trigger scroll handoff or overscroll. r=hiro

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4b0037f7b822