1847716 - Scrolling smoothly even with smooth scrolling disabled if you click and hold scrollbar

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[bier]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/116.0

Steps to reproduce:

Go to a long enough website and click and hold the scrollbar track just a bit above the downward arrow button.

Actual results:

Website is scrolled down a single page immediately with no smooth scrolling as it should, but then it starts scrolling down smoothly.

Expected results:

Website should scroll down page by page immediately without an animation.

Comment 1

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

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

Comment 2

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2267e2cb83212ad23130e6bc5f12b88eb2068cd0&tochange=22f915e74381a95195bd1f1b9cdc43d70fdcdf58

Comment 3

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

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

:rzvncj, since you are the author of the regressor, bug 1331390, could you take a look?

For more information, please visit BugBot documentation.

Comment 4

[Dan Robertson (:dlrobertson)]

I think we avoid an instant scroll because we unconditionally call ScrollTo here with ScrollMode::Instant. We should call ScrollTo with ScrollMode::Instant if StaticPrefs::general_smoothScroll() is not set.

Relevant files:

• layout/xul/nsSliderFrame.cpp

Comment 5

[Razvan Cojocaru]

(In reply to Dan Robertson (:dlrobertson) from comment #4)

I think we avoid an instant scroll because we unconditionally call ScrollTo here with ScrollMode::Instant. We should call ScrollTo with ScrollMode::Instant if StaticPrefs::general_smoothScroll() is not set.

Relevant files:

• layout/xul/nsSliderFrame.cpp

I see this has been marked as "good-first-bug", so I'll assume this is now better left strategically open for somebody new. Please let me know if you want this fixed ASAP instead.

Comment 6

[Johan Lorenzo [:jlorenzo]]

Regression triage: marking as fix-optional because this bug is tagged as good-first-bug.

Comment 7

[Botond Ballo [:botond]]

To explain how the regression came about: in bug 1331390, we effectively changed ScrollMode::Smooth to ScrollMode::SmoothMsd in the affected function (nsSliderFrame::PageScroll()), for reasons discussed in bug 1840025.

However, I overlooked at the time that the two modes differ in more than just the physics: the implementation of ScrollMode::Smooth supports either smooth or instant scrolling, while the implementation of ScrollMode::SmoothMsd unconditionally does smooth scrolling.

So, this would actually be fixed by going back to ScrollMode::Smooth as planned in bug 1840025.

However, until bug 1840025 is fixed, we can employ the shorter-term fix Dan suggested, and pass (StaticPrefs::general_smoothScroll() ? ScrollMode::SmoothMsd : ScrollMode::Instant).

Razvan, if you have a moment to prepare a fix like that, I'd say please go ahead; we are early in the release cycle and should be able to uplift that fix to 117, so that 116 is the only version affected by this regression.

(Separately, if someone with a Mac has a few spare cycles to look into the Mac-only test failure blocking the landing of bug 1840025, that would be greatly appreciated.)

Comment 8

[Razvan Cojocaru]

Attachment: Bug 1847716 - Don't scroll smoothly if smooth scrolling disabled when clicking and holding scrollbar. r?botond,dlrobertson

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/665bcec8a2d4
Don't scroll smoothly if smooth scrolling disabled when clicking and holding scrollbar. r=botond

Comment 10

[Serban Stanca [:SerbanS]]

Backed out for causing mochitest-plain failures in test_group_mouseevents.html.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_mouseevents.html | helper_scrollbartrack_click_overshoot.html | Scrollbar thumb hit

Comment 11

[Botond Ballo [:botond]]

Diagnosed the issue and suggested next steps in Phabricator.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc1080291948
Don't scroll smoothly if smooth scrolling disabled when clicking and holding scrollbar. r=botond

Comment 13

[Narcis Beleuzu [:NarcisB]]

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

Comment 14

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

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox117 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[Razvan Cojocaru]

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #14)

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox117 to wontfix.

For more information, please visit BugBot documentation.

Hello, I believe we do want the uplift (please see comment 7) but I'm not sure I'm authorized to make these types of changes. Handing this over to botond. :)

Comment 16

[Botond Ballo [:botond]]

Yep, I can make the request.

Comment 17

[Botond Ballo [:botond]]

Comment on attachment 9348109 [details]
Bug 1847716 - Don't scroll smoothly if smooth scrolling disabled when clicking and holding scrollbar. r?botond,dlrobertson

Beta/Release Uplift Approval Request

• User impact if declined: The "smooth scroll" preference is not respected when clicking on the scrollbar track and holding the mouse down.
• Is this code covered by automated tests?: No
• 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 fix is small, well-understood, and specific to code related to scrollbar track click-and-hold
• String changes made/needed:
• Is Android affected?: No

Comment 18

[Donal Meehan [:dmeehan]]

Comment on attachment 9348109 [details]
Bug 1847716 - Don't scroll smoothly if smooth scrolling disabled when clicking and holding scrollbar. r?botond,dlrobertson

Approved for 117.0b7

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b9a19c6d526

Comment 20

[Peter Magyari (Desktop QA)]

Reproducible on Firefox 116.0.2 (20230805021307).
Verified the fix on Beta 117.0b7 (20230813180142) and Nightly 118.0a1 (20230813213352).