1710324 - Always use non-native scrollbars on GTK

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Right now we paint scrollbars using gtk in nsNativeThemeGTK, and that makes them look a bit inconsistent if the non-native implementation ends up drawing them differently. I think scrollbars should look the same regardless of where they're drawn.

This will simplify code, and also bring proper scrollbar-{width,color} support to the native theme, which is nice because Thunderbird uses it if you use a theme there.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1710324 - Don't specify background color on native scrollbar elements on Windows / Linux. r=dao,Gijs

This allow the automatic scrollbar darkening logic to work without further
tweaks. We don't set appearance: none in any scrollbar on desktop (and we
support scrollbar-{width,color} to customize them anyways).

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1710324 - Tweak dark background detection so that it works for XUL use cases. r=mstange

Depends on D114696

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1710324 - nsNativeThemeGTK shouldn't need to be an nsIObserver. r=stransky

This is just cleanup. We have better ways to run code on shutdown (the pointer
to the theme object gets cleared on shutdown anyways).

Depends on D114697

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1710324 - Use non-native scrollbar drawing in nsNativeThemeGTK. r=stransky

Right now we paint scrollbars using gtk in nsNativeThemeGTK, and that
makes them look a bit inconsistent if the non-native implementation ends
up drawing them differently. I think scrollbars should look the same
regardless of where they're drawn.

This simplifies the code, and also brings proper scrollbar-{width,color}
support to the native theme, which is nice because Thunderbird uses it
if you use a theme there.

I opted for removing the code, but let me know if you'd rather keep it
behind the widget.non-native-theme.enabled pref or such.

Depends on D114698

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/238a4cfcdc86
Don't specify background color on native scrollbar elements on Windows / Linux. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/aa852464d828
Tweak dark background detection so that it works for XUL use cases. r=mstange
https://hg.mozilla.org/integration/autoland/rev/7328372519f1
nsNativeThemeGTK shouldn't need to be an nsIObserver. r=stransky

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for causing mass failures and crashes.

backout: https://hg.mozilla.org/integration/autoland/rev/51ac6b547af8ad310784863211e7f60d98836ff4

push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=7328372519f15c39d923f1ab7dfb2c0f24ae9136

failure logs (some):

1. REFTEST PROCESS-CRASH | dom/base/crashtests/499006-1.html | application crashed [@ nsTextPaintStyle::InitCommonColors()]
2. PROCESS-CRASH | dom/tests/mochitest/general/test_clipboard_events.html | application crashed [@ unsigned int nsIFrame::GetVisitedDependentColor<mozilla::StyleGenericColor<mozilla::StyleRGBA>, nsStyleBackground>(mozilla::StyleGenericColor<mozilla::StyleRGBA> nsStyleBackground::*)]
3. PROCESS-CRASH | /css/css-overflow/overflow-clip-001-crash.html | application crashed [@ RefPtr<mozilla::ComputedStyle>::operator->() const]
4. ==1938==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7fb4ca7c2c61 bp 0x7fff45c91290 sp 0x7fff45c91120 T0)
5. TEST-UNEXPECTED-FAIL | browser/modules/test/browser/browser_PartnerLinkAttribution.js | Uncaught exception - AbortError: Actor 'SpecialPowers' destroyed before query 'Spawn' was resolved
6. PROCESS-CRASH | /css/css-pseudo/active-selection-051.html | application crashed [@ RefPtr<mozilla::ComputedStyle>::operator->() const]
7. PROCESS-CRASH | dom/events/test/test_bug426082.html | application crashed [@ RefPtr<mozilla::ComputedStyle>::operator->() const]
8. REFTEST PROCESS-CRASH | layout/reftests/bugs/393649-1.html | application crashed [@ RefPtr<mozilla::ComputedStyle>::operator->() const]
9. PROCESS-CRASH | /input-events/input-events-cut-paste.html | application crashed [@ nsTextPaintStyle::InitCommonColors()]

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83b1cfc403ee
Don't specify background color on native scrollbar elements on Windows / Linux. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/93850ff5a79f
Tweak dark background detection so that it works for XUL use cases. r=mstange
https://hg.mozilla.org/integration/autoland/rev/d49f3f6ac128
nsNativeThemeGTK shouldn't need to be an nsIObserver. r=stransky
https://hg.mozilla.org/integration/autoland/rev/6f49d8893274
Use non-native scrollbar drawing in nsNativeThemeGTK. r=stransky

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/83b1cfc403ee
https://hg.mozilla.org/mozilla-central/rev/93850ff5a79f
https://hg.mozilla.org/mozilla-central/rev/d49f3f6ac128
https://hg.mozilla.org/mozilla-central/rev/6f49d8893274

Comment 9

[drJeckyll]

mozregression led me here, since today on Gentoo Linux running Firefox 90.0a1 (2021-05-12) (64-bit) with widget.non-native-theme.enabled to false, my scrollbars are missing arrows on both ends.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

widget.non-native-theme.gtk.scrollbar.allow-buttons=true probably fixes that, but I agree it's bogus that that pref has an effect if the non-native-theme is not enabled.

Comment 11

[drJeckyll]

Yep. That's fixed the issue for me.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Bug 1710873 should have the "real" fix.