Closed Bug 1846575 Opened 1 year ago Closed 1 year ago

Scrollbar scrolling is much slower in 116 compared to 115

Categories

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

Firefox 116
Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
119 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox-esr115 --- unaffected
firefox117 --- wontfix
firefox118 + affected
firefox119 --- fixed

People

(Reporter: therubex, Assigned: botond, NeedInfo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Scrollbar scrolling is much slower in 116 compared to 115

Load a fairly long page.
https://dslreports.com should suffice.

Place mouse cursor on the vertical scrollbar.
Hold down the left mouse button.

Expected.

You speedily scroll down the page.

Results.

You (relatively) slowly scroll down the page.

There is a large scroll speed difference between FF 115 & FF 116.

(Page Up/Page Down [keyboard key] scroll speed appears to be unaffected.)

Is the platform really Windows 7? Although Firefox 116 does not block running on Win7 yet, Win7 is not supported anymore.

I hope you don't mind me commenting here, but I have the same problem as the OP, I noticed it immediately after release 116 on Windows 10 x64.
I did the same steps, load any long page, click and hold the left mouse button on the scrollbar and watch as the scrolling speed significantly slows down, as if smooth scrolling is enabled, but I have it disabled in about:preferences.
Consequently the old ESR base doesn't have this problem, once the ESR build is updated to the new base it'll inherit this problem/quirk unless it is resolved.
Speaking of about:preferences, that page is long enough to notice the scrollbar slowdown.

Do you see the same slow scrolling using Safe mode? If so, would you be able to capture a profile of this issue? That gives us the data necessary to diagnose possible performance problems and regressions.

Flags: needinfo?(therubex)

As requested.

https://share.firefox.dev/3s0tp3Y

Data recorded on a clean profile (I hope the link works).

Problem persists in safe mode.

From the regression range (and lack of another obvious culprit in the profile), lets move this to the layout:scrolling component.
With the profile and regression range, I think we have enough info to move forward for the moment, so I'm clearing the need-info on the reporter.

Component: General → Layout: Scrolling and Overflow
Flags: needinfo?(therubex)
Product: Firefox → Core
Keywords: regression
Regressed by: 1331390

This issue has been fixed for me by Bug 1847716.

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

For more information, please visit BugBot documentation.

Flags: needinfo?(rzvncj)

Should be fixed by bug 1847716

Flags: needinfo?(rzvncj)

Bug 1847716 Fixed only if smooth scrolling is disabled.

If smooth scrolling is enabled, Firefox 116,117 scroll speed is x2 slower than Firefox 15 on Windows10.
Firefox 115: https://share.firefox.dev/44K5ELf
Firefox 117: https://share.firefox.dev/3qNlJBP

OS: Windows 7 → Windows
Flags: needinfo?(botond)

My first thought here was that maybe the speed difference is due to the difference between ScrollMode::Smooth and ScrollMode::SmoothMsd, and thus bug 1840025 will fix this, but based on some local experimentation, it's slower even with ScrollMode::Smooth.

It seems the speed difference somehow arises from our other changes in bug 1331390, i.e. using ScrollTo rather than ScrollByPage.

Perhaps multiple ScrollByPage calls accumulate a delta, and ScrollTo calls, if there is a previous scroll still in progress, would only add a page to the current scroll position, even if there is a scroll destination that is further along for a still happening scroll?

Yeah, I think that's right.

This even came up during the discussion of the fix for bug 1331390, when we were investigating a regression of the test helper_visual_scrollbars_pagescroll.html. Quoting from this comment:

the patch is regressing this test because the second click is processed while the smooth scroll for the first click is still in progress (such that we've only scrolled a fraction of the page), and we now do a ScrollTo to that intermediate scroll position + 1 full page, resulting in a total scroll distance of less than 2 pages.

There is a tension here between the goal of the patch (avoid overshoot) and what this test is trying to test (N clicks results in a scroll distance of N pages). On the one hand, a user may intend to scroll by precisely N pages (like say they're looking at a timeline of posts or pictures where each one takes up one page, and they want to advance by N posts), and expect that clicking N times on the scrollbar track will get them there. On the other hand, respecting this means that you can overshoot the cursor position if the thumb starts close to the cursor and you click on the track multiple times.

A reasonable compromise here is to keep the "avoid overshoot" logic for the "click and hold" case, but scroll by a full page per click in the "multiple clicks" case

The compromise we came up with failed to consider that even in the click-and-hold case, not scrolling a full page distance on each repeat of the timer has implications on how many repeats it takes to reach the destination, and thus the perceived speed of the scrolling.

We should be able to satisfy both goals here (scroll a full page per repeat timer tick, and avoid overshoot) if the logic to guard against overshoot factors in the destination of any currently ongoing smooth scroll.

A slight challenge is that we don't currently have easy access to that destination (it's not stored in nsHTMLScrollFrame::mDestination if the smooth scroll is handed off to APZ, and even in mApzSmoothScrollDestination it's only stored briefly until the request is dispatched to APZ). But that can be overcome (e.g. nsSliderFrame could track the destination itself).

Flags: needinfo?(botond)
Assignee: nobody → botond
Attachment #9350940 - Attachment description: Bug 1846575 - Scroll a full page page repeat timer tick during scrollbar track click-and-hold. r=rzvncj → Bug 1846575 - Scroll a full page per repeat timer tick during scrollbar track click-and-hold. r=rzvncj
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4397c50b3581 Introduce/rename some variables in nsSliderFrame::PageScroll for clarity. r=rzvncj https://hg.mozilla.org/integration/autoland/rev/1928c5d9696c Scroll a full page per repeat timer tick during scrollbar track click-and-hold. r=rzvncj
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 119 Branch

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

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

For more information, please visit BugBot documentation.

Flags: needinfo?(botond)

I do think we should uplift this fix to 118.

I'll give the patch a few more days of baking time on Nightly before making the request.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: