1977511 - Consider disabling overlay scrollbars if a `-webkit-scrollbar` style rule is specified with nonzero width and height

Status: ASSIGNED

(Core :: Layout: Scrolling and Overflow, enhancement, P3)

Description

[Daniel Holbert [:dholbert]]

We've got at least two cases where it seems like a Google site is trying to provide a thickness for scrollbars, which (in Safari and Edge with overlay scrollbars enabled) also successfully makes the scrollbars be "non-overlay-style", which happens to be a usability win in these cases:
bug 1836872
bug 1973196

I know we've already investigated and opted not to implement -webkit-scrollbar in general (bug 1432935). We might want to investigate doing a limited intervention for cases like this to at least take certain bundles of styles as a signal that we should switch to traditional scrollbars, though... Since that's how they end up working in Chromium and WebKit right now at least.

As an example, here's a testcase where the upper block uses scrollbars if they're enabled, and the lower two use ::-webkit-scrollbar to toggle non-overlay scrollbars with specified thicknesses in Chromium and WebKit:
https://jsfiddle.net/dholbert/L83ejpvd/

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

(In reply to Daniel Holbert [:dholbert] from comment #0)

As an example, here's a testcase where the upper block uses scrollbars if they're enabled, and the lower two use ::-webkit-scrollbar to toggle non-overlay scrollbars with specified thicknesses in Chromium and WebKit:
https://jsfiddle.net/dholbert/L83ejpvd/

Here's that fiddle as a bugzilla attachment, for convenience/better-archival.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (demonstrating the brokenness from nested scroll-containers, at e.g. Gmail)

Here's a testcase to demonstrate how this affects Gmail (bug 1973196).
A) In Firefox with overlay scrollbars, you can't scroll to reach SUCCESS. The inner area's overlay scrollbar isn't shown (it's covered up by the outer area's scrollbar, so you can't horizontally-scroll the inner area to see SUCCESS at all (unless you have some mechanism for touchpad/wheel-based horizontal scrolling).
B) In Firefox with legacy scrollbars, this is fine; you can scroll; you can see the two horizontal-scrollbars stacked and everything is fine.
C) In Chromium/WebKit, this is fine & you can scroll. This is true regardless of whether they're configured to use overlay scrollbars -- they react to the -webkit-scrollbar styles here by disabling overlay scrollbars and honoring the specified scrollbar width.

In this bug, I'm arguing that we should react to the prefixed scrollbar styles just like Chromium/WebKit do, and just activate legacy scrollbars (matching our (B) rendering).

We could do this on the level of the entire document, if that's easier to do (rather than turning overlay-scrollbars on/off for certain elements). Basically: whatever styles might deactivate overlay scrollbars in Chromium/WebKit should probably do the same for us (whether or not they also add additional fanciness in other browser-engines that we can ignore).

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screencast showing behaviors (A)/(B)/(C) on testcase 2

Here's a screencast showing the behaviors A/B/C from the previous comment.

Behavior (A) at the beginning is the problematic one -- that's where we're leaving content unreachable, basically.

Comment 4

[Alex Jakobi [:ajakobi]]

testcase 2 shows different behaviors between OSes.

On macOS (with default settings), we're always showing both toolbars with both being reachable and not placed on top of each other (on the z-axis). There seems to be no "always show scrollbars" setting available in the settings.

On Windows 11, I'm seeing the behavior as shown in the screencast in comment #3 with the setting "always show scrollbars" not available.

On Linux (Ubuntu 24.04, X11), I'm seeing the behavior as shown in the screencast in comment #3 with the setting "always show scrollbars" available.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

(In reply to Alex Jakobi [:ajakobi] from comment #4)

On macOS (with default settings), we're always showing both toolbars with both being reachable and not placed on top of each other (on the z-axis). There seems to be no "always show scrollbars" setting available in the settings.

On macOS this setting is in the OS settings: https://www.macrumors.com/how-to/make-scroll-bars-always-visible/

On Windows 11, I'm seeing the behavior as shown in the screencast in comment #3 with the setting "always show scrollbars" not available.

Same on Windows (https://www.windowscentral.com/how-always-show-scrollbars-windows-11)

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1977511 - Add reftests for ::-webkit-scrollbar. r?#layout-reviewers

There are 6 test cases;

1. webkit-scrollbar.html has
   ::-webkit-scrollbar { width: 8px; height: 8px; }
2. webkit-scrollbar-zero-width.html has
   ::-webkit-scrollbar { width: 0px; }
3. webkit-scrollbar-no-width-height.html has
   ::-webkit-scrollbar { background-color: #aaa; }
4. disable-horizontal-scrollbar-dynamically.html has no
   ::webkit-scrollbar styles, checks non-overlay scrollbars
   properly disappear once after the contents in the scroll container
   does no longer overflow
5. webkit-scrollbar-focus.html has
   :focus::-webkit-scrollbar { width: 8px; height: 8px; }
6. webkit-scrollbar-display-none.html has
   ::webkit-scrollbar { width: 8px; display: none; }

As of now 6) doesn't work as expected.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: WIP: Bug 1977511 - Disable overlay scrollbars if there's any non-zero width and height -webkit-scrollbar.

This change does;

1. not make the current <scrollbar> element be a ::-webkit-scrollbar
   pseudo element
2. rather, while traversing <scrollbar> element for its styling,
   additionally collect ::-webkit-scrollbar rules and match them
   as if the <scrollbar> is a ::-webkit-scrollbar pseudo
3. then if we found width > 0 or height > 0 styles on any
   non-root <scrollbar> element, disable overlay scrollbar globally per
   document, and kicks a media feature change evaluation

There are some restrictions;

0. The machinery for property restrictions doesn't work for
   ::-webkit-scrollbar since it's not implemented as a genuine psuedo
   element
1. ::-webkit-scrollbar is not applicable to the root scroll container
   I haven't figured out how to obtain the originating element, i.e. the
   <html> or <body>, while traversing the root <scrollbar>
2. Disabling overlay scrollbars does just work on the entire document,
   it doesn't disable overlay on an individual scrollbar. We may be able
   to do it by adding inline styles such as opacity: 1 ! important to
   each <scrollbar> to negate overlay styles defined in a UA style sheet
   [1] with -moz-overlay-scrollbar media query.

[1] https://searchfox.org/firefox-main/rev/58f365ba0eb5761a182f1925e4654cc75212b8ac/layout/style/res/scrollbars.css#140-149

Comment 8

[Hiroyuki Ikezoe (:hiro)]

I think I am being stuck at finding ways to apply property restrictions to ::-webkit-scrollbar styles.

The current approach (written in D280698) is to not make <scrollbar> be a ::-webkit-scrollbar, it just additionally collects ::-webkit-scrollbar styles while traversing <scrollbar>. The current D280698 works in most cases, but if there's any CSS property other than width and height for ::-webkit-scrollbar, the property is also applicable to <scrollbar>, thus it will break <scrollbar>, one observable result I have seen is an assertion for cached scrollbar styles hits.

Emilio, do you have opinions on this issue? I'd personally like the original approach that is to make <scrollbar> be a ::-webkit-scrollbar pseudo element since the machinery for property restrictions works as-it-is. And the current approach also has a couple of hacks in selector matching.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Yeah, adding a SetPseudoElement(webkitScrollbar) to the scrollbar node seems more straight-forward, but that said, that's gonna break the scrollbar caching as well. also, we should disable overlay scrollbars only on that particular scroll container.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1977511 - Use ScrollContainerFrame::UseOverlayScrollbars in ResizeObserver and ScrollbarAreaToExcludeFromCompositionBoundsFor. r?#layout-reviewers

Most of the remaining call sites of nsPresContext::UseOverlayScrollbars
for scrollbar parts' frames will be replaced by a new function in the
next change.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1977511 - Introduce nsLayoutUtils::UseOverlayScrollbars and use it in proper places instead of nsPresContext::UseOverlayScrollbars. r?#layout-reviewers

There remain some direct call sites of
nsPresContext::UseOverlayScrollbars.

0. two call sites in nsCSSFrameConstructor::GetAnonymousContent [0]
   These call sites for used for scrollbar style caching stuff,
   thoguh I am not 100% sure whether these two call sites needs to be
   replaced, in the next patch only width/height are applicable to
   ::-webkit-scrollbar pseudo elements and those properties are flagged
   as has_effect_on_gecko_scrollbars=false, so it's okay-ish?
   anyway the first one can not be replaced since it gets called before
   styling the <scrollbar> itself

1. in Element::GetClientAreaRect [1]
   it's for an optimization to avoid layout flush on the root scroll
   container, we will not apply ::-webkit-scrollbar to the root one, so it
   doesn't matter

2. in Gecko_GetScrollbarInlineSize [2]
   it's used only for internal CSS env scrollbar-inline-size, so it
   doesn't matter unless we use ::-webkit-scrollbar internally

3. in nsTreeBodyFrame::Init [3]
   it's XUL we won't use ::-webkit-scrollbar in chrome contents

4. in Gecko_MediaFeatures_UseOverlayScrollbars [4]
   it's used for -moz-overlay-scrollbars media query, we leave the
   styles in the media query, to disable overlay we will set active to
   the scrollbar element.

[0] https://searchfox.org/firefox-main/rev/33fd6bd39c625067a29f153adce6a4646e45750f/layout/base/nsCSSFrameConstructor.cpp#3964,4035
[1] https://searchfox.org/firefox-main/rev/e480af4adea0d838f97b105d7efdd32fa18b73c1/dom/base/Element.cpp#1057
[2] https://searchfox.org/firefox-main/rev/e480af4adea0d838f97b105d7efdd32fa18b73c1/layout/style/GeckoBindings.cpp#317
[3] https://searchfox.org/firefox-main/rev/e480af4adea0d838f97b105d7efdd32fa18b73c1/layout/xul/tree/nsTreeBodyFrame.cpp#293
[4] https://searchfox.org/firefox-main/rev/e480af4adea0d838f97b105d7efdd32fa18b73c1/layout/style/nsMediaFeatures.cpp#112

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1977511 - Resolve ::-webkit-scrollbar pseudo element styles soon after the scroll container style was set. r?#layout-reviewers

While traversing ::-webkit-scrollbar pseudo element for its styling,
additionally collect rules for <scrollbar> defined in UA style sheet
and match them.

There are two caveats;

1. we tell a lie in @supports selector as if we don't handle
   ::-webkit-scrollbar at all
2. only width and height properties are applicable to ::webkit-scrollbar
   pseudo elements, but these values are never used for <scrollbar>
   elements becasue of the nature of our scrollbar implementation, i.e.
   it rather respects scrollbar-width.

There's a restriction;

1. ::-webkit-scrollbar is not applicable to the root scroll container
   I haven't figured out how to obtain the originating element, i.e. the
   <html> or <body>, while traversing the root <scrollbar>

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1977511 - Disable overlay scrollbars if there's any non-zero width and height defined in ::-webkit-scrollbar. r?#layout-reviewers

To disable scrollbars on a given scrollbar we set active attribute to
the vertical and horizontal scrollbar elements as if the user is
hovering over the scrollbars so that we don't specifically need to
cancel out styles in @media (-moz-overlay-scrollbars) query in
scrollbars.css even though the media query is document wide.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Thank you, Emilio. I've somehow managed to make it happen. I am still not confident scrollbar style caching, I've written what I am thinks is in the commit message of D281571. I hope it does make sense to you!

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Hmm, looking a bit closer at this and all the special cases that were needed, I wonder if it'd be easier to just add a RefPtr<ComputedStyle> ScrollContainerFrame::mWebKitScrollbarStyle; and update it in DidSetComputedStyle, via ProbePseudoElementStyle... Seems that'd work and wouldn't be a penalty for pages that don't use ::-webkit-scrollbar (we don't selector-match / compute a style at all in ProbePseudoElementStyle...)

It might be faster than breaking the scrollbar style caching perhaps? Hiro, that seems like a maybe easier approach? I think we didn't consider this approach when discussing it (only either "adding more NAC" or "turning scrollbar element into ::-webkit-scrollbar").

Assuming it'd be fast enough to probe on a page with no ::-webkit-scrollbar rules, and that we only probe one style (::-webkit-scrollbar really has some extra pseudo-classes too like ::-webkit-scrollbar:horizontal etc, not sure if they work now), it might be faster and less invasive.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Thank you, Emilio! With the new approach the code change became much simpler!