1461538 - [GTK] Fix dark theme detection in GNOME 3.28

Status: RESOLVED DUPLICATE of bug 1527048

(Core :: Widget: Gtk, defect)

Description

[Evan Klitzke]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180512192648

Steps to reproduce:

Dark theme detection is broken in Firefox on GNOME 3.28 (the most recent release). I explained the issue in detail over at bug #70315 (comment 46, not sure how to link it here).

To re-summarize: before GNOME 3.28 there was a GTK option called gtk-application-prefer-dark-theme. This option no longer does anything, and as of GNOME 3.28 users are supposed to set their theme to "Adwaita dark" to get a dark theme. The patch I'm including here has a hard-coded check for Adwaita-dark, and sets the theme to Adwaita-light in that case.

This is kind of a hack, but I suspect it will fix the issue for most users.

Comment 1

[Evan Klitzke]

Attachment: gtk-dark-theme.patch

Comment 2

[QwertyChouskie]

This should probably check for any theme with "dark" in the name.

Comment 3

[Gingerbread Man]

(In reply to Evan Klitzke from comment #0)
> I explained the issue in detail over at bug #70315 (comment 46,
> not sure how to link it here).

The text bug 70315, comment 46 will result in a clickable link to the respective comment in another bug report.
Comment 0 by itself will link to a comment in the report you're looking at.

You can also put the full bug URL in the See Also field for bugs that are related but don't either depend or block others.

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8975693 [details] [diff] [review]
gtk-dark-theme.patch

Thanks for your patch. Please consider to use MozReview (http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html) for further patches, it makes the review process much easier.

+            // This is a hack to suppress dark themes on GNOME 3.28+, which
+            // doesn't use the gtk-application-prefer-dark-theme option.

Why do you think so? I don't see any indication that gtk-application-prefer-dark-theme won't work on GNOME 3.28+.

+            // Instead, we explicitly check for an Adwaita-dark theme, and set
+            // it to Adwaita-light.

I don't think we want such hard coded theme checks at Firefox code, especially when Adwaita is used by minority of Linux distros/DE, Ubuntu uses Radiance/Ambiance, Mint Mate, KDE has Arc and so on...

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think karlt is probably a better reviewer for this than I am, but I should see if Karl agrees.

Comment 6

[Bruno Pagani]

I agree that hardcoding a particular theme is a bad idea, themes like Arc, Adapta and co. are quite popular too, and for instance KDE (but virtually any non-GNOME DE) has its own default GTK theme (Breeze in this case).

Comment 7

[Evan Klitzke]

I think it would be straightforward to come up with a dark theme detection heuristic provided the default foreground and background colors for the theme. However, it wasn't clear to me how to do this using the existing GTK APIs (a lot of the theme APIs are marked as deprecated, and I don't grok the CSS APIs).

Comment 8

[Martin Stránský [:stransky] (ni? me)]

(In reply to Evan Klitzke from comment #7)
> I think it would be straightforward to come up with a dark theme detection
> heuristic provided the default foreground and background colors for the
> theme. However, it wasn't clear to me how to do this using the existing GTK
> APIs (a lot of the theme APIs are marked as deprecated, and I don't grok the
> CSS APIs).

Bug 1411425 may be a solution here.

Comment 9

[Karl Tomlinson (:karlt)]

I'm happy to let Martin decide.  He has spent more time thinking about this
than I.  The best options are not clear.  For example, some people actually
want dark themes in content

I initially agree with Martin.  I don't see much value adding a check for just
Adwaita-dark, because that doesn't seem a whole lot more likely than other
dark themes.  If someone has gone to the trouble of changing the theme name to
Adwaita-dark, then I'd guess they are just as likely to pick another theme.

If themes seem to be following some sort of convention re the -Dark suffix in
their name, then adding a heuristic would probably be reasonable.
However, I'm not clear there is a good way to test whether a theme
with a particular non "-Dark" name is actually available.

Adwaita is one of the few themes that we absolutely know for sure will be
available.  Perhaps that lends some weight to implementing just Adwaita-dark.
But given there is a pref to let people choose anyway, I'm not sure this
case is worthwhile.

I do expect the long term solution will be removing at least most of the
natively themed elements, and instead using a built-in CSS theme.  With that,
I'm concerned about foreign-looking and foreign-behaving scrollbars, but it
may eventually become a necessity with sandboxed content processes.  If that
is the long term direction, then fiddling with heuristics in the meantime
might be just delaying more important work.

I'm curious about the claim in
https://bugzilla.mozilla.org/show_bug.cgi?id=70315#c46 that GNOME Tweaks "is
probably how most users had set" gtk-application-prefer-dark-theme.  Does
GNOME not provide any UI for theme variants?

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to Evan Klitzke from comment #7)
> I think it would be straightforward to come up with a dark theme detection
> heuristic provided the default foreground and background colors for the
> theme.

An heuristic may be feasible, but there is still the question of which light
theme to use if a dark theme is detected.

Bear in mind that applying the heuristic globally would still only be useful
sometimes because some themes have dark backgrounds for some elements and
light backgrounds for others.

Comment 11

[Bruno Pagani]

(In reply to Karl Tomlinson (:karlt) from comment #9)
> Adwaita is one of the few themes that we absolutely know for sure will be
> available.

And why would that be the case? I don’t have Adwaita on my system.

Comment 12

[Karl Tomlinson (:karlt)]

(In reply to Bruno Pagani from comment #11)

Adwaita is built into more recent versions of GTK+ libraries.  It is not included in old versions.
So you are correct to imply that Adwaita is not always available, but we can
detect the GTK version, and so there are some situations where we know it is available.

Comment 13

[QwertyChouskie]

(In reply to Karl Tomlinson (:karlt) from comment #9)
> I'm happy to let Martin decide.  He has spent more time thinking about this
> than I.  The best options are not clear.  For example, some people actually
> want dark themes in content
> 
> I initially agree with Martin.  I don't see much value adding a check for
> just
> Adwaita-dark, because that doesn't seem a whole lot more likely than other
> dark themes.  If someone has gone to the trouble of changing the theme name
> to
> Adwaita-dark, then I'd guess they are just as likely to pick another theme.
> 
> If themes seem to be following some sort of convention re the -Dark suffix in
> their name, then adding a heuristic would probably be reasonable.

"-Dark" or "-dark" seem to be common enough for it to be worth implementing
(might also add "-Nokto" for Adapta variants)

> However, I'm not clear there is a good way to test whether a theme
> with a particular non "-Dark" name is actually available.

I assume GTK would return an error if a theme is not available. 

> Adwaita is one of the few themes that we absolutely know for sure will be
> available.

Assuming GTK+ 3.14 or later.  Raleigh can be used as the fallback for older versions.

> Perhaps that lends some weight to implementing just Adwaita-dark.
> But given there is a pref to let people choose anyway, I'm not sure this
> case is worthwhile.
> 
> I do expect the long term solution will be removing at least most of the
> natively themed elements, and instead using a built-in CSS theme.  With that,
> I'm concerned about foreign-looking and foreign-behaving scrollbars, but it
> may eventually become a necessity with sandboxed content processes.  If that
> is the long term direction, then fiddling with heuristics in the meantime
> might be just delaying more important work.
> 
> I'm curious about the claim in
> https://bugzilla.mozilla.org/show_bug.cgi?id=70315#c46 that GNOME Tweaks "is
> probably how most users had set" gtk-application-prefer-dark-theme.  Does
> GNOME not provide any UI for theme variants?

Comment 14

[Martin Stránský [:stransky] (ni? me)]

I think this is pretty much covered by Bug 1527048. Closing as a duplicate, please reopen if you want to do more work on this.