985267 - Australis menu bar hard to read on Windows 7 dark third-party themes

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Jake Z]

Attachment: NightlyMenuBar_Active.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140318030202

Steps to reproduce:

Use a Windows 8 RT dark theme on a Windows 7 (x64) machine. Open up the Nightly build. Right-click near the top of the window and check "Menu Bar". View the lack-of-contrast in the menu bar.


Actual results:

The menu bar has very little contrast for the text and the background. It's relatively difficult to read and is very unsightly to say the least.


Expected results:

The menu bar should have been clearly readable.

I've checked https://bugzilla.mozilla.org/show_bug.cgi?id=863862, in which I observe the actual opposite happening: the inactive menu is more readable than the active menu.

This is similar to the following bug, yet that bug is marked as Windows XP-specific:
https://bugzilla.mozilla.org/show_bug.cgi?id=925156

I should note that I've used this theme without problems on the current Firefox release. It only appears to be an issue with Australis.

Comment 1

[Jake Z]

Attachment: NightlyMenuBar_Inactive.png

Attached the inactive menu bar image.

Comment 2

[Matthew N. [:MattN]]

I think the problem is that we can't adjust the menu bar fog based on the system theme colours using CSS-alone. We would need to write code to figure out the colour value and then do something like LightweightThemes and figure out if that colour is "light" or "dark".

I think we can probably fix this more easily by moving this rule to only apply when -moz-windows-default-theme is true since the original commit said this was just for Glass. Do third-party Win7 themes use glass-like transparency? Does anyone use them?

An easier fix would be to remove or reduce the opacity of this fog now that we have the glass fog from the tabs.

Marking P3- as this can stop the menubar from being usable, especially for people who need more contrast.

Comment 3

[Matthew N. [:MattN]]

We could also just make the text color black (instead of the current CaptionText/InactiveCaptionText) like we do for background tabs in this case IIRC.

Comment 4

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #3)
> We could also just make the text color black (instead of the current
> CaptionText/InactiveCaptionText) like we do for background tabs in this case
> IIRC.

This. We should just not be using CaptionText for these wherever we have the fog (but we *should* be using it elsewhere, see the XP bugs mentioned originally)

Comment 5

[:Gijs (he/him)]

Attachment: don't use captiontext when we have menubar fog (in compositor),

This should work. Checked that it disabled the rules on win8 with compositor. I still don't know exactly what makes this fog, but I've verified that it isn't there in aero basic, so I believe that 'not compositor' should cover it correctly.

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8393552 [details] [diff] [review]
don't use captiontext when we have menubar fog (in compositor),

Seems like we'd fall back to MenuText without these rules. That's not guaranteed to be visible on the hardcoded white background either.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8393552 [details] [diff] [review]
don't use captiontext when we have menubar fog (in compositor),

No, we don't. See http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#131

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8393552 [details] [diff] [review]
don't use captiontext when we have menubar fog (in compositor),

The code you linked to won't change the MenuText color from menu.css, as that targets the menu elements directly.

I guess you could just use color:inherit here to pick up the right color from the toolbar.

Comment 9

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 8393552 [details] [diff] [review]
> don't use captiontext when we have menubar fog (in compositor),
> 
> The code you linked to won't change the MenuText color from menu.css, as
> that targets the menu elements directly.
> 
> I guess you could just use color:inherit here to pick up the right color
> from the toolbar.

Blargh. I totally blame the devtools inspector, which doesn't show these rules when inspecting the menu element in the inspector view, just the browser.css ones - oddly enough, the rules do show up under the 'computed style' view. Go figure.

Comment 10

[:Gijs (he/him)]

(filed bug 985547 about the devtools issue, new patch coming up)

Comment 11

[:Gijs (he/him)]

Attachment: don't use captiontext when we have menubar fog (in compositor),

How is this?

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8393602 [details] [diff] [review]
don't use captiontext when we have menubar fog (in compositor),

>+/* Make the menu inherit the toolbar's color. On non-compositor (Aero Basic, XP modern, classic)
>+ * this is defined above. Otherwise (Aero Glass, Windows 8), this is hardcoded to black in
>+ * browser-aero.css. */
>+#main-menubar > menu {
>+  color: inherit;
>+}

nit: add :not(:-moz-lwtheme), as your comment also only applies in that case. menu.css takes care of :-moz-lwtheme.

Comment 13

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/63cf491f2032

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63cf491f2032

Comment 15

[:Gijs (he/him)]

Comment on attachment 8393602 [details] [diff] [review]
don't use captiontext when we have menubar fog (in compositor),

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: menu bar text is hard to read on some themes, harder to uplift the win 8 titlebar color work
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 16

[:Gijs (he/him)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/56bce5bf9979
https://hg.mozilla.org/releases/mozilla-beta/rev/46c9e3f63657

Comment 17

[u279076]

I think this should get some QA testing since we had several complaints around this and similar issues.

Comment 18

[Cornel Ionce [:noni]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0

Verified using several Windows 8 RT dark themes on Windows 7 64bit with Firefox 31 beta 5(build ID: 20140626181429).
The Australis menu bar is now visible when Active.