1700169 - Scrollbar thumb should be squished during overscroll

Status: VERIFIED FIXED

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

Description

[Markus Stange [:mstange]]

Steps to reproduce:

1. Set apz.overscroll.enabled to true.
2. Scroll-drag into overscroll, up and down.

Expected results:
The scrollbar thumb should become shorter as the overscroll amount increases.

Actual results:
The scrollbar thumb position and length stays fixed during overscroll.

---

We can do this by setting a scale transform on the scrollbar thumb in AsyncCompositionManager. Search for ApplyAsyncTransformToScrollbarForContent in attachment 8832590 [details] [diff] [review].
Unfortunately this will also squish the circle endcaps of the scrollbar thumb. Ideally, we would shorten the thumb while keeping the endcaps perfectly circle round. But I think that's something we can worry about as a follow-up (if ever).

Comment 1

[Botond Ballo [:botond]]

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

Unfortunately this will also squish the circle endcaps of the scrollbar thumb. Ideally, we would shorten the thumb while keeping the endcaps perfectly circle round. But I think that's something we can worry about as a follow-up (if ever).

This part is basically the same issue as bug 1642494, and (if solved at all) would likely have the same solution.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I just noticed Chrome Canary on Mac does not change the scrollbar thumb size at all, it changes the scroll bar's position, say if you overscroll 10px at the top edge, the scrollbar starts off (the right edge, 10px). It's a release version of Chrome in the video in bug 1702977, so they've changed their behavior, and I think moving the scrollbar position kind of makes sense to me.

Comment 4

[Markus Stange [:mstange]]

The native behavior is to squish. I would much prefer to align with native scroll views, than with something that Chrome chose.

Comment 5

[Mehmet]

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

I just noticed Chrome Canary on Mac does not change the scrollbar thumb size at all, it changes the scroll bar's position, say if you overscroll 10px at the top edge, the scrollbar starts off (the right edge, 10px). It's a release version of Chrome in the video in bug 1702977, so they've changed their behavior, and I think moving the scrollbar position kind of makes sense to me.

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

The native behavior is to squish. I would much prefer to align with native scroll views, than with something that Chrome chose.

fyi: Both, not to squish the scrollbars and to change the scrollbars position during overscroll are current bugs in Chrome and are not the intended behavior.

Thank you in advance for implementing the native macOS scroll behavior in Firefox :)

Comment 6

[Botond Ballo [:botond]]

Markus, I know you were interested in this. Would you like to give this build a whirl, and let me know if the amount of squishing of the thumb feels appropriate to you?

Comment 7

[Markus Stange [:mstange]]

Attachment: strange squishing in profile manager listbox

Looks and feels great, thanks!

The only place where I ran into some trouble was the profile manager, see attached video. But on regular web pages it works great.

And I've also noticed slight imperfections in list boxes. For example, in this list testcase, when overscrolling at the top, the scrollbar moves a bit upwards during the squishing, and when overscrolling at the bottom, its bottom end also moves a bit upwards during squishing.

data:text/html,<select size="8" style="width:150px; padding: 5px;"><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option><option>Hello</option></select>

Comment 8

[Markus Stange [:mstange]]

Oh, here's a real website where the squishing is bad: https://nnethercote.github.io/perf-book/heap-allocations.html#profiling
Overscrolling horizontally on the right edge in the nested scroll frame with the DHAT output causes the horizontal scrollbar to be improperly squished (its right edge moves leftwards).

Comment 9

[Botond Ballo [:botond]]

Thanks for testing!

It looks like the claim here that "the origin with respect to which the scale is applied is the origin of the entire scrollbar, rather than the origin of the scroll thumb", which was added back in bug 1088984, and which my fix relies on, is a lie. It actually appears to be the origin of the page, or something like that.

For the root scroll frame, there is no difference, and so far this code has only been exercised for the RSF (since only the RSF can be async-zoomed and thus have its scroll thumbs scaled in the compositor), so no one noticed. But now that I'm using it for subframes as well, it makes a difference, and the computation needs to be fixed to account for the correct transform origin.

Comment 10

[Botond Ballo [:botond]]

(I can't seem to get the profile manager to overscroll on Linux at all, but I assume it's affected by the same problem.)

Comment 11

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Factor out the code to scale the thumb into its own function. r=tnikkel

This includes the translation to ensure the scaling keeps the position
of the top of the thumb constant.

Comment 12

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Take into account the correct transform origin when scaling the thumb. r=tnikkel

The code was written assuming the transform origin is the top
of the scrollbar track, but it's actually the layer tree origin.

So far, scaling was only done for thumbs of the root scroll frame
where there is no difference, so no one noticed.

Depends on D114453

Comment 13

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Extend ScaleThumbBy() to allow keeping the position of either the start or the end of the thumb constant. r=tnikkel

Depends on D114454

Comment 14

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Squish scroll thumbs when overscrolled. r=tnikkel

Depends on D114455

Comment 15

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Apply the test async properties in GetOverscrollAmount(). r=tnikkel

Depends on D114456

Comment 16

[Botond Ballo [:botond]]

Attachment: Bug 1700169 - Add a reftest. r=tnikkel

Depends on D114457

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d6bd327a9b1
Factor out the code to scale the thumb into its own function. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/a00c3552ae0a
Take into account the correct transform origin when scaling the thumb. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/8be5c79181f3
Extend ScaleThumbBy() to allow keeping the position of either the start or the end of the thumb constant. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/fe4f63525d0b
Squish scroll thumbs when overscrolled. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/2ce685277c8c
Apply the test async properties in GetOverscrollAmount(). r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/fcaa9068465d
Add a reftest. r=tnikkel

Comment 18

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/3d6bd327a9b1
https://hg.mozilla.org/mozilla-central/rev/a00c3552ae0a
https://hg.mozilla.org/mozilla-central/rev/8be5c79181f3
https://hg.mozilla.org/mozilla-central/rev/fe4f63525d0b
https://hg.mozilla.org/mozilla-central/rev/2ce685277c8c
https://hg.mozilla.org/mozilla-central/rev/fcaa9068465d

Comment 19

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on latest Firefox Nightly 90.0a1 (2021-05-11) (64-bit) on MacOS 10.15. Hey Botond, do we intend to Uplift this in 89?

Comment 20

[Botond Ballo [:botond]]

(In reply to Timea Cernea [:tbabos] from comment #19)

Hey Botond, do we intend to Uplift this in 89?

I don't think we need to uplift this to 89. It's more of a visual polish item, and the patches could probably use some baking on the nightly channel.

Comment 21

[Timea Cernea [:tbabos][inactive]]

Thanks, Botond! Updating the status to verified-fixed based on Comment 19 and Comment 20.