Closed Bug 1818702 Opened 2 years ago Closed 2 years ago

Scrollbar keeps re-stretching while the page is scrolled by keyboard's arrow keys

Categories

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

Product:
Core
Component:
Layout: Scrolling and Overflow
Platform:
Desktop
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
112 Branch
Tracking Flags:
Tracking Status
firefox-esr102 --- unaffected
firefox110 --- unaffected
firefox111 --- fixed
firefox112 + fixed

People

(Reporter: csasca, Assigned: botond)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Bug 1818702 - Handle a non-default device scale correctly in ScrollThumbUtils. r=rzvncj
48 bytes, text/x-phabricator-request
Details | Review
Bug 1818702 - Use reftest-zoom instead of layout.css.devPixelsPerPx in test. r=emilio
48 bytes, text/x-phabricator-request
Details | Review

Found in

  • Firefox 111.0b5

Affected versions

  • Firefox 111.0b5
  • Firefox 112.0a1

Tested platforms

  • Affected platforms: macOS 10.15.7 -> macOS 13.2
  • Unaffected platforms: Windows, Ubuntu

Steps to reproduce

  1. Launch Firefox
  2. Access any scrollable page of choice or this PDF example
  3. Scroll the page with up/down arrows

Expected result

  • The scrollbar is fixed while scrolling the page

Actual result

  • The scrollbar keeps re-stretching after every press on the arrows / space key

Regression range

  • Will see for a regression

Additional notes

  • The issue can be seen in the following attachment
  • This doesn't happen if the page is scrolled by any other method (touchpad, dragging the scrollbar with the mouse, etc).

Seems to happen on any page. Using a screen where the css px to device pixel ratio is not 1 seems to be key to reproducing.

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

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

For more information, please visit auto_nag documentation.

status-firefox110: --- → unaffected
status-firefox-esr102: --- → unaffected
Flags: needinfo?(rzvncj)

Unfortunately I don't have access to a macOS machine. Requesting info from Botond Ballo.

Flags: needinfo?(rzvncj) → needinfo?(botond)

[Tracking Requested - why for this release]: We should probably figure out how deep of an issue is this. From comment 1 it might be just a scaling issue.

tracking-firefox111: --- → ?

FWIW, as suggested by Botond on Matrix I've tried to make this happen on Linux using layout.css.devPixelsPerPx - but I couldn't, at least not with 1.0 and 2.0 as values. I'm either missing something or there's something else macOS-specific involved that I can't simulate.

I don't think this is macos specific, I can reproduce on Windows too. And there is a similar sounding bug reported on Windows, bug 1818721.

OS: macOS → All
Summary: [macOS] Scrollbar keeps re-stretching while the page is scrolled by keyboard's arrow keys → Scrollbar keeps re-stretching while the page is scrolled by keyboard's arrow keys

If we don't get a forward fix soon I think we should consider backing out bug 1554795 and uplifting the backout. We have a short window to avoid shipping this bug now that it's on beta. My rationale is that keyboard scrolling (where this bug happens) is more common then pinch zooming on desktop (where bug 1554795 happens).

(In reply to Timothy Nikkel (:tnikkel) from comment #7)

I don't think this is macos specific, I can reproduce on Windows too. And there is a similar sounding bug reported on Windows, bug 1818721.

They may well be related, but my only Firefox development environment at this point is Linux, where neither of them is reproducible in any way I could think of.

I can reproduce this in a Windows VM, on http://www.pdf995.com/samples/pdf.pdf, with layout.css.devPixelsPerPx set to 1.5.

I don't have a development environment set up on my Windows VM, nor the disk space to set one up, but I can run a build with added logging there.

I'm also curious what codepath differs between Windows/Mac vs. Linux that's relevant here.

I can reproduce on Linux fwiw (with overlay scrollbars and a devicePixelRatio of 2). On wayland, which uses per-monitor DPI unlike X11 (maybe related?)

After playing around with it some more, I can also reproduce it on Linux, with layout.css.devPixelsPerPx set to 1.5, though the effect is very subtle and I had to watch it very closely to notice anything (it's basically like a 1-pixel perturbation in the length of the scrollbar as it scrolls, or something like that).

It seems to be independent of Wayland or whether the scrollbars are overlay, but it does seem to be sensitive to the value of layout.css.devPixelsPerPx (e.g. for me it reproduces with 1.5 but not 2.0), as well as to the height of the browser window (e.g. if you resize the browser window to different heights, it will reproduce at some heights and not at others).

It's much more noticeable then that on macos and windows,

Here's a minimal fix that should be enough to avoid backing out bug 1554795: https://treeherder.mozilla.org/jobs?repo=try&revision=9415eecbdbc9a68d839909268e11c49a45271d76

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

Here's a minimal fix that should be enough to avoid backing out bug 1554795: https://treeherder.mozilla.org/jobs?repo=try&revision=9415eecbdbc9a68d839909268e11c49a45271d76

Catalin, Timothy, could you kindly check if the build in this Try push (direct link to Mac build here) resolves the issue for you?

Flags: needinfo?(tnikkel)
Flags: needinfo?(catalin.sasca)

Hey Botond. just tried the mac build you provided on macOS 13.2.1 and it seems that there still is a bit of a skip on the scrollbar, though not so visible as before. Here's a screencast with the behavior on the try build.

Flags: needinfo?(catalin.sasca)

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

After playing around with it some more, I can also reproduce it on Linux, with layout.css.devPixelsPerPx set to 1.5, though the effect is very subtle and I had to watch it very closely to notice anything (it's basically like a 1-pixel perturbation in the length of the scrollbar as it scrolls, or something like that).

It seems to be independent of Wayland or whether the scrollbars are overlay, but it does seem to be sensitive to the value of layout.css.devPixelsPerPx (e.g. for me it reproduces with 1.5 but not 2.0), as well as to the height of the browser window (e.g. if you resize the browser window to different heights, it will reproduce at some heights and not at others).

You're right. Happens on X11 as well (I'm on X11). I just assumed that 2.0 would be enough to test, but as you've noticed it made no difference. 1.5 does indeed cause a very very small and quick stretch.
Sorry for the confusion!

(In reply to Catalin Sasca, QA [:csasca] from comment #16)

Hey Botond. just tried the mac build you provided on macOS 13.2.1 and it seems that there still is a bit of a skip on the scrollbar, though not so visible as before. Here's a screencast with the behavior on the try build.

I echo this, definitely less obvious, but definitely still there.

Flags: needinfo?(tnikkel)

The bug is marked as tracked for firefox111 (beta). We have limited time to fix this, the soft freeze is in 9 days. However, the bug still isn't assigned and has low severity.

:fgriffith, could you please find an assignee and increase the severity for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Flags: needinfo?(fgriffith)

Are we going to backout the regressor for beta, if we're not going to blackout do we think the potential patch will also fix Big 1818721?

Flags: needinfo?(tnikkel)

Donal, can we back out bug 1554795 on Beta (111) while keeping it on Nightly (112)? I think the fix I posted + one more follow-up fix should address the regression on 112 before the end of the Nightly cycle, but for 111 it seems safer to back out the regressor and avoid uplifts late in the Beta cycle, the original fix is not that pressing.

Flags: needinfo?(tnikkel)
Flags: needinfo?(dmeehan)
Flags: needinfo?(botond)

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

Donal, can we back out bug 1554795 on Beta (111) while keeping it on Nightly (112)? I think the fix I posted + one more follow-up fix should address the regression on 112 before the end of the Nightly cycle, but for 111 it seems safer to back out the regressor and avoid uplifts late in the Beta cycle, the original fix is not that pressing.

Yes, we can backout bug 1554795 on beta only. I'll include the backout in my uplifts tomorrow.
Thanks

Flags: needinfo?(dmeehan)
Assignee: nobody → botond
Severity: S4 → S2
Flags: needinfo?(fgriffith)
Priority: -- → P2

(In reply to Timothy Nikkel (:tnikkel) from comment #18)

(In reply to Catalin Sasca, QA [:csasca] from comment #16)

Hey Botond. just tried the mac build you provided on macOS 13.2.1 and it seems that there still is a bit of a skip on the scrollbar, though not so visible as before. Here's a screencast with the behavior on the try build.

I echo this, definitely less obvious, but definitely still there.

Just to clarify: the remaining issue is no longer a "stretch" (incorrect scrollbar length), just a small jitter in the scrollbar position, right?

I see the scrollbar size change on this bugzilla page on macOS when scrolling with the touchpad. During scrolling the scrollbar has size A, and then once it starts fading out, it jumps to a slightly bigger size B. So if you're towards the top of the document, you see the scrollbar's bottom end grow downwards; if you're towards the bottom, you see the upper end grow upwards; and if you're in the middle of the document, you see both ends grow outwards.

Actually, experimenting more, it seems that the scrollbar is small whenever the scroll offset hasn't changed since the previous drawn frame and large whenever the scroll offset has changed since the previous frame. I can see the the large size whenever a new main thread paint happens, for example if I slowly scroll over a link with hover feedback.

(In reply to Markus Stange [:mstange] from comment #24)

I see the scrollbar size change on this bugzilla page on macOS when scrolling with the touchpad. During scrolling the scrollbar has size A, and then once it starts fading out, it jumps to a slightly bigger size B. So if you're towards the top of the document, you see the scrollbar's bottom end grow downwards; if you're towards the bottom, you see the upper end grow upwards; and if you're in the middle of the document, you see both ends grow outwards.

Actually, experimenting more, it seems that the scrollbar is small whenever the scroll offset hasn't changed since the previous drawn frame and large whenever the scroll offset has changed since the previous frame. I can see the the large size whenever a new main thread paint happens, for example if I slowly scroll over a link with hover feedback.

If you get a chance to try the build from comment 15, it would be nice to confirm that there are no size (length) changes in this build.

Ah, I had missed that. Yes, I can confirm that there are no size changes in the build from comment 15. But the position indeed still shifts. There is no shifting if the scroll position is at the top of the page, and there maximal shifting if the scroll position is at the end of the page. It feels like the incorrect scrollbar position is calculated with respect to a too-long scrollbar. Is it not using the nsSliderFrame's mRatio for the calculation?

(In reply to Markus Stange [:mstange] from comment #26)

Is it not using the nsSliderFrame's mRatio for the calculation?

Sort of. Since mRatio is in part a function of the pinch-zoom level, to handle async zooming correctly we replicate the mRatio calculation in the compositor. I think the symptoms suggest that our calculation is slightly inaccurate, or possibly fails to take into account the device scale somewhere.

Setting 111 to fixed as the regressor, Bug 1554795, was backed out of beta
Moving the tracking+ to 112

status-firefox111: affected → fixed
tracking-firefox111: + → ---
tracking-firefox112: --- → +

There were two problems:

  • The line scroll amount was queried in LayoutDevice pixels
    and used as CSS pixels without applying the device scale.

  • The thumb length was rounded in CSS pixels. To match main
    thread behaviour, it needs to be rounded in LayoutDevice
    pixels.

Blocks: 1819848
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2aa4e168b236 Handle a non-default device scale correctly in ScrollThumbUtils. r=rzvncj

https://hg.mozilla.org/mozilla-central/rev/2aa4e168b236

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox112: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bbe1109a1f94 Use reftest-zoom instead of layout.css.devPixelsPerPx in test. r=emilio

Backed out for causing reftest failures in gfx/layers/apz/test/reftest.

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: REFTEST TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/reftest/async-scrollbar-1-v-fullzoom.html == gfx/layers/apz/test/reftest/async-scrollbar-1-v-fullzoom-ref.html | image comparison, max difference: 18, number of differing pixels: 20
Flags: needinfo?(botond)
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6ab34dd7226f Use reftest-zoom instead of layout.css.devPixelsPerPx in test. r=emilio
Flags: needinfo?(botond)

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

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

Attachment

General

Creator:
Created:
Updated:
Size: