1812868 - Expose scrollbar-inline-size as a CSS variable to chrome code

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, enhancement)

Description

[Alessandro Castellani [:aleca]]

Since the entire tree list container is scrollable and the table header is in a sticky position, the scrollbar goes on top of it.
This is a bit annoying when the OS scrollbar has a "floating" style and it's selected, which ends up covering a good part of the column picker button when selected.

We should add a padding at the end of the column picker button based on an environmental variable detecting the width of the expanded OS scrollbar.
E.g.: padding-inline-end: env(scrollbar-inline-size);

Gentle ping to Emilio to see if it's possible to get something like that.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1812868 - Expose scrollbar-inline-size as a CSS variable to chrome code. r=mstange

For that we need to:

• Make GetDPIRatioForScrollbarPart thread-safe: This was using the
  widget for bug 1727289, but just looking at the print preview scale
  is enough to fix that.

• Make nsPresContext::UseOverlayScrollbars() thread-safe: We store the
  RDM pane stuff in the pres context.

The rest is pretty straight-forward.

TODO: Make the test actually work.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7d370501c50
Expose scrollbar-inline-size as a CSS variable to chrome code. r=mstange

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07c689de250c
fix mismatch in scrollbar size. r=emilio CLOSED TREE

Comment 4

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e7d370501c50
https://hg.mozilla.org/mozilla-central/rev/07c689de250c

Comment 5

[Alessandro Castellani [:aleca]]

Thanks Emilio for adding this feature.
If I'm reading the code correctly, scrollbar-inline-size is only exposed to chrome and not content. If that's the case, I don't think we can use it since the new about3Pane is in a browser, so it's all content.

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d63352149381
Backed out 2 changesets for causing bug 1817539. CLOSED TREE

Comment 7

[Sandor Molnar[:smolnar]]

Backed out for causing crashes in gfx/tests/crashtests/1652750-deep-scene-stack.html (see bug 1817539)

Backout link (central): https://hg.mozilla.org/mozilla-central/rev/d63352149381cb6abd8b83f06daa6a596bae036c

Push with failures

Failure log

Autoland backout: https://hg.mozilla.org/integration/autoland/rev/d63352149381cb6abd8b83f06daa6a596bae036c

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Alessandro Castellani [:aleca] from comment #5)

Thanks Emilio for adding this feature.
If I'm reading the code correctly, scrollbar-inline-size is only exposed to chrome and not content. If that's the case, I don't think we can use it since the new about3Pane is in a browser, so it's all content.

As long as the document uri scheme is chrome:// it should work, so I don't think that's an issue.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Tim, I don't understand how this patch can cause bug 1817539 tbh, any ideas?

I'm not touching any display-list code, nor tweaking stuff in a way that could cause more stack space to be used... The weirdest of things is that this only happens on opt builds rather than debug (or both).

No worries if you don't have any idea off-hand tho, I'll figure out. I'm splitting the patch up and trying to find the culprit, but I'm confused. Also it seems this was already on file as bug 1771983 (but only happened once, which is really weird).

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Somehow https://hg.mozilla.org/try/rev/d2a026c25ce96b79892e43cebb7a99fe5ec11732 causes this. I'm a bit baffled, but DL building does call into that quite a bit, maybe the compiler is optimizing something in a bad way?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

I moved nsPresContext::Theme() out of line and that fixed it, but it does seem like that test is a bit of a time bomb...

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Given I made the effort to split the patch, I'll just land them split up, since I believe it's better.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e989416b614
Tweak RDM and theme setup. r=mstange
https://hg.mozilla.org/integration/autoland/rev/808a91c49890
Tweak scrollbar scaling set-up. r=mstange
https://hg.mozilla.org/integration/autoland/rev/0e53db091ffb
Expose chrome-only environment variables to all chrome:// documents, not just chrome-docshells. r=mstange
https://hg.mozilla.org/integration/autoland/rev/6fd4ae812ff2
Expose scrollbar-inline-size as a chrome-only environment variable. r=mstange
https://hg.mozilla.org/integration/autoland/rev/4f75949dc3d7
Add tests. r=mstange

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1e989416b614
https://hg.mozilla.org/mozilla-central/rev/808a91c49890
https://hg.mozilla.org/mozilla-central/rev/0e53db091ffb
https://hg.mozilla.org/mozilla-central/rev/6fd4ae812ff2
https://hg.mozilla.org/mozilla-central/rev/4f75949dc3d7

Comment 15

[Timothy Nikkel (:tnikkel)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Somehow https://hg.mozilla.org/try/rev/d2a026c25ce96b79892e43cebb7a99fe5ec11732 causes this. I'm a bit baffled, but DL building does call into that quite a bit, maybe the compiler is optimizing something in a bad way?

So we think this is a miscompilation that you were just unlucky to have tripped over? What followup actions should we look into?

Comment 16

[Emilio Cobos Álvarez (:emilio)]

It doesn't seem a miscompilation, just that the optimizer probably got more aggressive in a way that it increased the stack frame size of BuildDisplayList just enough to trigger the stack overflow.

Comment 17

[Sören Hentzschel]

This is a bit annoying when the OS scrollbar has a "floating" style and it's selected, which ends up covering a good part of the column picker button when selected.

This is still the case (tested on macOS 13.2). It looks like these patches added platform support for scrollbar-inline-size, but it still has to be used, right? Should this ticket be re-opened?

Comment 18

[Wayne Mery (:wsmwk)]

I also still see this when using 2023-02-28 build on mac

Comment 19

[Alessandro Castellani [:aleca]]

The work done on this bug was exclusively to introduce a scrollbar-inline-size environmental variable we can use in CSS.
I'm updating the bug title to reflect that, but the bug should also be moved into Core to be more accurate.
Creating a follow up to start using that variable.

Comment 20

[Timothy Nikkel (:tnikkel)]

Just leaving a comment to note that bug 1824565 is about similar stackoverflow crashes on win32 that were experienced here.