Closed Bug 1272332 Opened 8 years ago Closed 8 years ago

Allow dark gtk theme with environment variable

Categories

(Core :: Widget: Gtk, defect)

Product:
Core
Component:
Widget: Gtk
Version:
47 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox46 --- unaffected
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- verified

People

(Reporter: jonny, Assigned: supertux88)

References

Details

(Keywords: nightly-community, regression)

Attachments

(4 files, 4 obsolete files)

screenshot showing firefox nightly with a light theme, and a second gtk app using the system dark theme
(deleted), image/png
Details
Screenshot from 2016-06-08 22-18-06.png - shows Firefox using a dark GTK theme
(deleted), image/png
Details
Allow dark gtk theme with environment variable
(deleted), patch
karlt
: review+
ritu
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
Details | Diff | Splinter Review
comparison on browser window scrollbar with and without e10s enabled
(deleted), image/png
Details 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160512030253

Steps to reproduce:

Updated Firefox nightly


Actual results:

It changed from using the gtk3 dark theme to gtk3 light theme


Expected results:

It should have continued to use the system gtk3 dark theme
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Confirmed on current nightly on Archlinux and Gnome 3.20.
Blocks: gtk3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [nightly-community]
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Keywords: regression
Blocks: 1216658
No longer blocks: gtk3
Yes, this is intentional. At the moment, using the theme-provided colors breaks a lot of web content, due to web authors not expecting the default colors to be inverted when they partially style objects such as text areas (ending up with a lot of dark text on dark backgrounds and white text on white backgrounds).
Until we figure out a way to use a separate color scheme for web content and for the browser's UI, it is preferable to ignore the system dark theme.
If I’m happy to put up with the occasional hard-to-read text, is there anyway I can revert this change, maybe and `about:config` setting?
(In reply to Jonny Barnes from comment #3)
> If I’m happy to put up with the occasional hard-to-read text, is there
> anyway I can revert this change, maybe and `about:config` setting?

I have something in mind that might just work for the separate styling of content/ui that I want to try first. If I can't make it work quickly I'll add a knob like an about:config pref.
If e10s is enabled, this lets us have the dark theme for the chrome, and no dark theme for web content. The dark theme is still disabled if e10s is disabled, though.
Assignee: nobody → nical.bugzilla
Attachment #8753289 - Flags: review?(karlt)
I assume this patch isnt in the latest nightly?
Comment on attachment 8753289 [details] [diff] [review]
Don't disable the dark theme in content processes

Nice, thanks!
Attachment #8753289 - Flags: review?(karlt) → review+
(In reply to Jonny Barnes from comment #6)
> I assume this patch isnt in the latest nightly?

Not yet but it will be soon.
And Valgrind. Just plan to run a full set of Linux builds/tests on Try before attempting to re-push :)
https://treeherder.mozilla.org/logviewer.html#?job_id=28121553&repo=mozilla-inbound#L22430
I guess BrowserTabsRemoteAutostart() || XRE_IsContentProcess() has side effects when not e10s.  BrowserTabsRemoteAutostart() is documented as a getter.
Perhaps one of these is not expecting to be called so early.
BrowserTabsRemoteEnabled is usually called for the first time later at startup and does some lazy initialization, while XRE_IsParentProcess and friends just comapare against a global that is set right at the beginning of the process. This version is green on try.
Flags: needinfo?(nical.bugzilla)
Attachment #8756305 - Flags: review?(karlt)
Attachment #8753289 - Attachment is obsolete: true
Comment on attachment 8756305 [details] [diff] [review]
simpler version that doesn't call BorwserTabsRemoteEnabled()

># HG changeset patch
># User Nicolas Silva <nsilva@mozilla.com>
># Parent  829d3be6ba648b838ee1953fdfa1a477dace752f
>Bug 1272332 - Honor gtk's global dark theme for the chrome when e10s is enabled. r=karlt

>-    // Disable dark theme because it interacts poorly with widget styling in
>-    // web content (see bug 1216658).

>+    if (!XRE_IsContentProcess()) {
>+        // Disable dark theme in processes that have web content because it
>+        // interacts poorly with widget styling (see bug 1216658).
>+        // To avoid triggering reload of theme settings unnecessarily, only set the
>+        // setting when necessary.

This is disabling the dark theme in the parent process, isn't it?
Attachment #8756305 - Flags: review?(karlt) → review-
(In reply to Karl Tomlinson (ni?:karlt) from comment #17)
> 
> This is disabling the dark theme in the parent process, isn't it?

*facepalm* Yes this patch is totally doing the opposite of what I intended it to do.
That means this is a regression in 47?
Version: 49 Branch → 47 Branch
(In reply to Milan Sreckovic [:milan] from comment #21)
> That means this is a regression in 47?

Not a regression in the sense that we use generally (as in "this CSS property renders incorrectly since 47"). When Firefox was using gtk2 we never used the global dark theme. When we switched to gtk3 we automatically started picking up the global dark theme colors for a few release, but disabled it eventually because it was breaking too much web content.

With e10s we have the possibility to enable the dark theme for the chrome process without enabling it for content processes, which is great. The issue right now is that we need to know whether e10s is enabled when choosing what to do with the gtk theme and the latter happens very early at startup, too early to query the e10s prefs/settings without causing some breakage, apparently.

So I'm keen on waiting for e10s to get enabled by default and allow using the dark theme colors for the chrome process whenever that happens (disabling e10s will cause some web content to break again but it won't be the default). In the mean time, we are just defaulting to the behavior Firefox had for many years.
Can you add an option in about:config to use the dark theme again? I liked firefox with the dark theme.
Yes, even if this does cause some problems with certain content, if users like it, you should at least give them an options in about:config to use it if they really want, if you don't have it enabled by default. You could think of it as an experimental feature.
Perhaps the setting could be only overridden when GTK_THEME is not in the environment, to provide a mechanism to override the override.
(In reply to Benjamin Neff from comment #23)
> Can you add an option in about:config to use the dark theme again? I liked
> firefox with the dark theme.

My first attempt did something like that. The problem is that this is happening early at startup when the code that manage about:config prefs isn't properly initialized yet, and it caused the patch to get backed out. An environment variable could do the job, but it isn't very ergonomic so people tend to not use the features that are gated that way.
In this case it's simple to add so if an environment variable switch works for you I can add it. let me know.
An environment variable switch would do for me. Couldn't you though just have a setting in about:config which requires Firefox to be restarted for changes to take effect?
environment variable is ok, as long as I have a possibility to use the dark theme, I'm happy :)
Has this been fixed, I’ve just installed a new GTK3 dark theme called Arc-Dark and now Firefox is also dark!
I suspect the prefs are available.

This is the call stack I see.
(There is confusion due to nsLookAndFeel::Init() and nsXPLookAndFeel::Init()
being called at different times.)

nsLookAndFeel::Init() (nsLookAndFeel.cpp:1142)
nsXPLookAndFeel::GetInstance() (nsXPLookAndFeel.cpp:264)
mozilla::LookAndFeel::GetInt(mozilla::LookAndFeel::IntID, int*) (nsXPLookAndFeel.cpp:912)
GetInt (LookAndFeel.h:561)
nsChromeRegistryChrome::CheckForOSAccessibility() (nsChromeRegistryChrome.cpp:160)
ScopedXPCOMStartup::SetWindowCreator(nsINativeAppSupport*) (nsAppRunner.cpp:1620)
XREMain::XRE_mainRun() (nsAppRunner.cpp:4121)

CheckForOSAccessibility() uses prefs immediately after LookAndFeel:

http://searchfox.org/mozilla-central/rev/3f096f780166bc4772199d79a79c8e3541ff99be/chrome/nsChromeRegistryChrome.cpp#169
(In reply to Benjamin Neff from comment #23)
> Can you add an option in about:config to use the dark theme again? I liked
> firefox with the dark theme.

(In reply to Benjamin Neff from comment #28)
> environment variable is ok, as long as I have a possibility to use the dark
> theme, I'm happy :)

(In reply to Karl Tomlinson (ni?:karlt) from comment #25)
> Perhaps the setting could be only overridden when GTK_THEME is not in the
> environment, to provide a mechanism to override the override.

I don't think it is a good idea to (re-)add a broken feature (dark theme support) to have it enabled by people who deliberately want to break their firefox UI. As much as I want dark theme support, we have to wait for e10s for this to make sense.
It doesn't actually break all sites, just some, or at least out of all the sites I went to, only a few were affected and not actually so badly, though I couldn't sometimes see text because it was the same colour as the background I could still highlight it to see it, and it was only really in search boxes or occasionally text boxes of other sorts.

I think that if a user thinks it is more important to them to have a dark Firefox than one where they don't need to highlight text from time to time, then you should give them the option even if it doesn't work perfectly right now. Or at least give advanced users some way of doing it even it's not through the interface and it's just an environment variable or something.

If you still won't do it though, what's the schedule for this e10 thing? When does it look like that will be the default?
> to have it enabled by people who deliberately want to break their firefox UI.

I don't want to break my firefox UI, because it was not broken before. I created a chrome/userContent.css file in my firefox profile to style the input-fields for broken websites. And this worked very well for over a year now (until dark themes stopped working in firefox).

I'm perfectly fine with an environment variable, because I think it is an advanced feature that needs additional changes (to make websites work with dark themes), but it is NOT a broken feature. It does exactly what it should do: use the dark gtk theme.
Keywords: nightly-community
Whiteboard: [nightly-community]
I'm also looking for a way to re-enable dark-theme as it is perfectly functional and preferred by me with custom userContent.css.  I can understand the desire to have made honoring the local theme a non-default, given the customization required to workaround broken websites with half-specified colors. Arguably it should have been initially introduced in that way with gtk3, but that boat sailed a while ago.

Now to have hard-disabled support for the dark theme in the 47 release, with no configuration option, was heavy handed and unnecessarily disruptive. Its a regression that should be fixed sooner with something simple like a non-default about:config flag or lacking that, environment var. If this "e10s" thing lands later and obsolesces the need for custom userContent.css, then great.
Just chiming in to say that I miss the dark theme, but having read the bugs about it, I get why it's disabled temporarily. I look forward to its return, but I totally get mozilla's reasoning about this one. 

I'm just happy to see them thinking about this and doing work for us Linux folks.
<rant>
Completely agreed with the previous comments. While disabling the dark property by default was probably a sensible move, two things are completely unacceptable:
1. Not a word about the change in the changelog, even though it is an obviously user-visible change (however small the number of impacted users may be).
2. Not a single way to re-enable the behaviour, through an environment variable or a command-line flag.

Having a better way to handle dark themes in e10s is all well and good, but that's in no way a reason to disable dark themes unconditionally. As already mentioned, it is reasonably easy to make Firefox usable with dark themes, and this an actual feature (compared to e.g. Chromium that never bothered with the dark property).
</rant>

Now, there is fortunately a way to work around this brutal move, without reverting the patch and recompiling (which is not something I want to do on such a big codebase). It is very easy to override GTK3 themes, so my workaround is to create a new theme that is always dark regardless of the "dark" property. For instance, with Adwaita, copy the theme in ~/.themes/Adwaita-dark, and symlink gtk-3.0/gtk.css to gtk-3.0/gtk-dark.css. Then use "GTK_THEME=Adwaita-dark firefox" to launch Firefox, I personally create an executable script "/usr/local/bin/firefox" to make this system-wide, with the following contents:

> #!/bin/sh
> GTK_THEME=Xfce-dusk-Adwaita-dark /usr/bin/firefox "$@"

But again, I don't find acceptable that this hack is the least invasive way to have Firefox dark again.
(In reply to Jonny Barnes from comment #29)
> Created attachment 8761380 [details]
> Screenshot from 2016-06-08 22-18-06.png - shows Firefox using a dark GTK
> theme
> 
> Has this been fixed, I’ve just installed a new GTK3 dark theme called
> Arc-Dark and now Firefox is also dark!

See my previous comment, it works because this theme is always dark (whether or not the "dark" GTK property is set). Thanks by the way, your comment inspired my workaround ;)
(In reply to Kevin Brodsky from comment #36)
> <rant>
> Completely agreed with the previous comments. While disabling the dark
> property by default was probably a sensible move, two things are completely
> unacceptable:

Guys, I appreciate your passion and your involvement on bugzilla, but please remember that doing everything right takes more time that it seems and that we have limited engineering resources here. We can only spend a limited amount of time fixing gtk dark themes when at the same time a driver bug or whatnot is causing hundreds of thousands of users to see their browser crash at startup every day. These users would certainly find it completely unacceptable that I or any other contributor spend any time looking into dark theme issues on Linux rather than their own problem.

Notifying us of issues and helping us understand what matters for a large enough amount of users is useful, but ranting on bugzilla, while it feels good at the moment of posting, only discourages contributors from fixing your problem rather than someone else's.
Even this my own comment is off topic (apologies).

We'll get there, probably not the ideal way, never soon enough. In the mean time, thanks for pointing bugs out, please keep doing that and please refrain from emitting judgement on bugzilla.

I'll try a few things out whenever I have time.
(In reply to Nicolas Silva [:nical] from comment #38)
> Guys, I appreciate your passion and your involvement on bugzilla, but please
> remember that doing everything right takes more time that it seems and that
> we have limited engineering resources here. We can only spend a limited
> amount of time fixing gtk dark themes when at the same time a driver bug or
> whatnot is causing hundreds of thousands of users to see their browser crash
> at startup every day. These users would certainly find it completely
> unacceptable that I or any other contributor spend any time looking into
> dark theme issues on Linux rather than their own problem.
> 
> Notifying us of issues and helping us understand what matters for a large
> enough amount of users is useful, but ranting on bugzilla, while it feels
> good at the moment of posting, only discourages contributors from fixing
> your problem rather than someone else's.
> Even this my own comment is off topic (apologies).
> 
> We'll get there, probably not the ideal way, never soon enough. In the mean
> time, thanks for pointing bugs out, please keep doing that and please
> refrain from emitting judgement on bugzilla.
> 
> I'll try a few things out whenever I have time.

Just to be clear, I realise that this issue is not a priority, and that it takes time to address it properly. In fact, I do not blame any Firefox dev because dark themes are not properly handled, this is definitely a difficult and time-consuming problem. I only condemn the very harsh way the problem has been put under the carpet: disabling the dark property by default is fine, because this is a lesser evil for most users, but not including a single way for Firefox to behave as expected from a GTK application is unnecessarily radical and unexpected from many users. As a previous comment showed, many of these users will probably get very confused and start using GTK3 themes that are always dark, regardless of "gtk-application-prefer-dark-theme", which just adds more problems to the equation.

Now, quitting the rant for good, I suggest to read an environment variable, because it is easy to just ignore it afterwards, when dark theme handling becomes good enough to be enabled by default. Apart from the possible security implication (I don't see any in this case, but I can be wrong), this looks like a trivial modification to the code that currently resets the dark property unconditionally.
Attached patch Allow dark themes behind a pref (obsolete) (deleted) — — Splinter Review 
This try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=7389d90c30de059e665480809f9bc110d7041033 seems to indicate that you were right about the pref. It is possible to call into them this early at startup, and it was most likely the telemetry stuff in BrowserTabsRemoteAutostart() that caused the failures in my previous attempt.

The pref is "widget.allow-gtk-dark-theme", off by default.
Attachment #8756305 - Attachment is obsolete: true
Attachment #8763495 - Flags: review?(karlt)
Attachment #8763495 - Flags: review?(karlt) → review+
Comment on attachment 8763495 [details] [diff] [review]
Allow dark themes behind a pref

>+    if (isContentProcess || !allowDarkTheme) {

I suggest leaving isContentProcess out in this iteration.
Someone with e10s may prefer to force dark themes even in content.
(In reply to Karl Tomlinson (ni?:karlt) from comment #41)
> I suggest leaving isContentProcess out in this iteration.
> Someone with e10s may prefer to force dark themes even in content.

I'd rather not, since mixing light and dark color schemes in web pages never does what one would expect. On the other hand I don't know if we'll be able to keep the two (content theme and chrome theme) separate in the future if we move rendering to the compositor process.
I'll land it without isContentProcess for now.
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f052f1a6a05
Allow gtk the global dark theme behind a pref. r=karlt
(In reply to Nicolas Silva [:nical] from comment #42)
> I'd rather not, since mixing light and dark color schemes in web pages never
> does what one would expect. On the other hand I don't know if we'll be able
> to keep the two (content theme and chrome theme) separate in the future if
> we move rendering to the compositor process.
> I'll land it without isContentProcess for now.

Thanks a lot, glad to see that you could add a pref for this in the end. Regarding isContentProcess, maybe it would be even better to have two separate prefs, one for content and the other for chrome? I *think* that what most people really want is to have just the chrome dark, not the content, so as to avoid the multiple hacks needed to get readable widgets. Now I don't know if that would work with e10s, that's just a suggestion.
https://hg.mozilla.org/mozilla-central/rev/0f052f1a6a05
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
I tested this patch and it doesn't work as expected, so I tried some debugging. Somehow 'mozilla::Preferences::GetBool("widget.allow-gtk-dark-theme", false);' always returns false, independently of what I set in about:config. I didn't debug the GetBool further, but you mentioned already that it doesn't work properly early at startup? Can you please have a look at this again?
(In reply to Benjamin Neff from comment #46)
> I tested this patch and it doesn't work as expected, so I tried some
> debugging. Somehow
> 'mozilla::Preferences::GetBool("widget.allow-gtk-dark-theme", false);'
> always returns false, independently of what I set in about:config. I didn't
> debug the GetBool further, but you mentioned already that it doesn't work
> properly early at startup? Can you please have a look at this again?

Reopening based on this.

See also bug 1216658 comment 42 and below... this patch doesn't seem like a complete solution to this bug even if it worked as advertised.
Status: RESOLVED → REOPENED
status-firefox50: fixed → affected
Resolution: FIXED → ---
(In reply to Kevin Brodsky from comment #36)
> <rant>
Justified rant is justified. There are people this affected. We were so happy to /finally/ have proper GTK theme integration with 46, and then they go and nix half of it without telling anybody...

> my workaround is to create a new theme that is always dark regardless of the
> "dark" property.
I did the same to work around LightDM's lack of dark theme support!

> Then use "GTK_THEME=Adwaita-dark firefox" to launch Firefox
You can also force firefox to use the a dark theme from a .desktop file. Override /usr/share/applications/firefox.desktop by copying it to ~/.local/share/applications/firefox.desktop. Add the environment variable to the "Exec" line(s):
...
Exec=env GTK_THEME=Theme-dark firefox %u
...

If you have weird text colors with the forced dark theme, you might un-symlink its Theme-dark/gtk-3.0/settings.ini and make a new one, inverting dark and normal colors. To avoid confusing myself later, I also gave it its own name, etc in Theme-dark/index.theme. It's a lot of trouble to have to go to.
Flags: needinfo?(milan)
Flags: needinfo?(milan)
(In reply to Dão Gottwald [:dao] from comment #47)
> (In reply to Benjamin Neff from comment #46)
> > I tested this patch and it doesn't work as expected, so I tried some
> > debugging. Somehow
> > 'mozilla::Preferences::GetBool("widget.allow-gtk-dark-theme", false);'
> > always returns false, independently of what I set in about:config. I didn't
> > debug the GetBool further, but you mentioned already that it doesn't work
> > properly early at startup? Can you please have a look at this again?
> 
> Reopening based on this.

I'm gonna back this out in the next few days so that we don't show the broken pref in about:config.
(In reply to Dão Gottwald [:dao] from comment #49)
> (In reply to Dão Gottwald [:dao] from comment #47)
> > (In reply to Benjamin Neff from comment #46)
> > > I tested this patch and it doesn't work as expected, so I tried some
> > > debugging. Somehow
> > > 'mozilla::Preferences::GetBool("widget.allow-gtk-dark-theme", false);'
> > > always returns false, independently of what I set in about:config. I didn't
> > > debug the GetBool further, but you mentioned already that it doesn't work
> > > properly early at startup? Can you please have a look at this again?
> > 
> > Reopening based on this.
> 
> I'm gonna back this out in the next few days so that we don't show the
> broken pref in about:config.

https://hg.mozilla.org/integration/fx-team/rev/e82080019746
Landed a few hours ago: https://hg.mozilla.org/mozilla-central/rev/e82080019746
Target Milestone: mozilla50 → ---
I created a patch to allow the dark theme with the "MOZ_ALLOW_GTK_DARK_THEME" environment variable, because the pref didn't work.

This is my first patch, would appreciate feedback.
Attachment #8778483 - Flags: review?(karlt)
Comment on attachment 8778483 [details] [diff] [review]
Allow dark gtk theme with environment variable

Thank you
Attachment #8778483 - Flags: review?(karlt) → review+
Can someone merge this? Or what are the next steps?
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1cf1b0ff308
Allow dark gtk theme with environment variable. r=karlt
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f1cf1b0ff308
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hi Benjamin,
Since this bug is a regression and also affects 49/50, do you want to uplift this for 49/50 if this patch is not too risky?
Flags: needinfo?(mozilla)
Hi Gerry,
I think this patch should be safe to uplift for 49/50.
Flags: needinfo?(mozilla)
Hi Benjamin,
Please create an uplift request for 49/50. Thanks.
Flags: needinfo?(mozilla)
Comment on attachment 8778483 [details] [diff] [review]
Allow dark gtk theme with environment variable

Approval Request Comment
[Feature/regressing bug #]: 1216658
[User impact if declined]: Linux users using dark GTK themes would not be able to use their dark themes by setting MOZ_ALLOW_GTK_DARK_THEME
[Describe test coverage new/current, TreeHerder]: N/A
[Risks and why]: Very small - only adds an additional if condition to a check that only gets done if GTK users have a dark theme enabled
[String/UUID change made/needed]: N/A

Please note that the changes causing the issue mentioned in this bug were done by decision in bug 1216658 and this patch only adds a flag to revert to the old behavior if the user decides to. As discussed with :gchang on IRC, I am not sure if this qualifies for an uplift.

Creating the request for Benjamin since this is his first patch as a contributor and he does not have the permissions to edit flags...
Flags: needinfo?(mozilla)
Attachment #8778483 - Flags: approval-mozilla-beta?
Attachment #8778483 - Flags: approval-mozilla-aurora?
Assignee: nical.bugzilla → mozilla
Summary: [gtk3]Firefox Nightly has stopped using the system GTK3 dark theme → Allow dark gtk theme with environment variable
Attachment #8763495 - Attachment is obsolete: true
Attachment #8753784 - Attachment is obsolete: true
Hello Jonny, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(jonny)
Comment on attachment 8778483 [details] [diff] [review]
Allow dark gtk theme with environment variable

Stabilized on Nightly for a week and hides gtk dark theme behind an env variable, Aurora50+
Attachment #8778483 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Ritu Kothari (:ritu) from comment #61)
> Hello Jonny, could you please verify this issue is fixed as expected on a
> latest Nightly build? Thanks!

This works as expected now :)
Flags: needinfo?(jonny)
I looked at the code and tried to make this work by editing my .desktop file and adding a variable in about:config, but no dice. Now that this is in nightly, can somebody provide a howto for making it work?
(In reply to mlissner from comment #65)
> I looked at the code and tried to make this work by editing my .desktop file
> and adding a variable in about:config, but no dice. Now that this is in
> nightly, can somebody provide a howto for making it work?

I tested it by opening a terminal and running the command `MOZ_ALLOW_GTK_DARK_THEME=true /opt/firefox/firefox`. To make that permanent you need to modify your .desktop file and add that MOZ...=true to the start of the Exec entry.
OK, yeah, that did it. Not sure why that didn't work before. Anyway, here's the tweak I added to ~/.local/share/applications/firefox-trunk.desktop:

Exec=env MOZ_ALLOW_GTK_DARK_THEME=true /home/mlissner/bin/firefox/firefox-bin %u

Seems to work, thanks!
(In reply to Jonny Barnes from comment #64)
> (In reply to Ritu Kothari (:ritu) from comment #61)
> > Hello Jonny, could you please verify this issue is fixed as expected on a
> > latest Nightly build? Thanks!
> 
> This works as expected now :)

Fantastic! Thank you for a prompt reply. :)
Status: RESOLVED → VERIFIED
status-firefox51: fixed → verified
Comment on attachment 8778483 [details] [diff] [review]
Allow dark gtk theme with environment variable

Review of attachment 8778483 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes a regression and was verified. Take it in 49 beta. Should be in 49 beta 7.
Attachment #8778483 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Was this supposed to be fixed in FF49? Because it surely isn't in FF50 stable, as soon as e10s is enabled I get the useless light default GTK3 scrollbar, barely distinguishable from a light web content background; environment variable is set.
Works fine for me even with e10s and several content processes, as long as MOZ_ALLOW_GTK_DARK_THEME is defined when I launch Firefox, of course.
Yeah, the problem is window scrollbars, I realized later that it won't be possible to theme them any more because of process sandboxing, as far as I understand they reside in web content space, so the browser will use a general style for those.
Hopefully I will figure out something to put in userchrome.css to get the same visual result of any other properly themed GTK3 window, but no dice so far, is it even possible?
That's weird because for me the scrollbars are exactly the same as in other GTK3 applications... I don't really know the interaction with userchrome.css though.
This is a screenshot comparing the vertical windows scrollbars with Firefox 50.1.0, with and without e10s enabled, in the former case they are drawn according to the configured GTK3 theme like any other window, with e10s I have a general light default style (and no scroll buttons).

The rest of the window is themed properly, the scrollbars are correctly rendered while showing non web content, e.g.: the about:config page.
For me, scrollbars for the whole content inside the tab are consistent with the GTK3 theme. But I think I got what you're talking about. If the page contains a form without custom styling for scrollbars (such as this very comment form), then indeed I observe that with e10s (and only with e10s) they are always styled with the default light GTK3 theme.

I think this is actually expected: web content is not supposed to be styled according to the system theme. This is what has been causing havoc with dark themes, as many websites enforce e.g. a black font or a white background for widgets. From what I understood, only styling the chrome may be supported with e10s, and I think it's actually a good thing.
(In reply to Kevin Brodsky from comment #76)
> For me, scrollbars for the whole content inside the tab are consistent with
> the GTK3 theme. But I think I got what you're talking about. If the page
> contains a form without custom styling for scrollbars (such as this very
> comment form), then indeed I observe that with e10s (and only with e10s)
> they are always styled with the default light GTK3 theme.
> 
> I think this is actually expected: web content is not supposed to be styled
> according to the system theme. This is what has been causing havoc with dark
> themes, as many websites enforce e.g. a black font or a white background for
> widgets. From what I understood, only styling the chrome may be supported
> with e10s, and I think it's actually a good thing.

+1, that's very good. web content must not get styled by system themes because that will break many websites.
Those are the browser window scrollbars, not strictly web content, but somehow they are now because of sandboxing.
Again the tab scrollbar is themed normally for me even with e10s. How did you enable e10s? I have:
- browser.tabs.remote.force-enable = true
- dom.ipc.processCount = 2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: