Closed Bug 1004576 Opened 6 years ago Closed 6 years ago

[Australis] Background tabs are unreadable when using a dark Windows theme

Categories

(Firefox :: Theme, defect)

29 Branch
All
Windows 8.1
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 32
Tracking Status
firefox29 + wontfix
firefox30 + verified
firefox31 --- verified
firefox32 --- verified
b2g-v1.4 --- fixed

People

(Reporter: ashughes, Assigned: jaws)

References

Details

(Whiteboard: [Australis:P2])

Attachments

(2 files)

Attached image M35Poqb.png
Disclaimer: This bug may be related or a dupe of bug 950664.

We've been getting a lot of feedback from users updating to Firefox 29 with dark Windows themes not being able to see their background tabs. I've attached a screenshot from a user which shows this effect. Both IE and Chrome use a dark grey background for background tabs which makes this more easy to read.

The steps to reproduce this are fairly trivial:
1. Install Firefox 29 or above
2. Open the Windows Control Panel to Appearance > Personalization > Color and Appearance
3. Select the grey color swatch and expand Show Color Mixer
4. Slide brightness all the way to the left to make it Black
5. Click Save changes and start Firefox

I'm sure there's an easier way to trigger this (ie. installing a dark theme) but I wasn't able to find a real-world example to test. Perhaps someone can provide some real world examples.

One final note, User Advocacy believes this issue likely warrants a chemspill.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #0)
> One final note, User Advocacy believes this issue likely warrants a
> chemspill.

Given that, let's make sure it's nominated for tracking. :)
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #0)
> Created attachment 8415982 [details]
> M35Poqb.png
> 
> Disclaimer: This bug may be related or a dupe of bug 950664.
> 
> We've been getting a lot of feedback from users updating to Firefox 29 with
> dark Windows themes not being able to see their background tabs. I've
> attached a screenshot from a user which shows this effect. Both IE and
> Chrome use a dark grey background for background tabs which makes this more
> easy to read.
> 
> The steps to reproduce this are fairly trivial:
> 1. Install Firefox 29 or above
> 2. Open the Windows Control Panel to Appearance > Personalization > Color
> and Appearance
> 3. Select the grey color swatch and expand Show Color Mixer
> 4. Slide brightness all the way to the left to make it Black
> 5. Click Save changes and start Firefox


This isn't trivial to reproduce, because with these steps, I can't reproduce, and neither can Jared.

It might be a dupe of bug 1004482.
Depends on: 907373
Ehm, I can?

Jim, is there a way we can 'read' the windows 8 window border color? Or maybe even have a chrome-only CSS flag/ pseudo-class called `-moz-windowborder-dark` and `-moz-windowborder-light`, or something?
Flags: needinfo?(jmathies)
(In reply to Mike de Boer [:mikedeboer] from comment #3)
> Ehm, I can?
> 
> Jim, is there a way we can 'read' the windows 8 window border color? Or
> maybe even have a chrome-only CSS flag/ pseudo-class called
> `-moz-windowborder-dark` and `-moz-windowborder-light`, or something?

That's covered by bug 578780. We haven't had a lot of luck getting at the glass color in win7 but according to bug 578780, comment 22 there's a call we can make on Win8 to get the frame color.
Flags: needinfo?(jmathies)
Duplicate of this bug: 1004482
(In reply to Jim Mathies [:jimm] from comment #4)
> (In reply to Mike de Boer [:mikedeboer] from comment #3)
> > Ehm, I can?
> > 
> > Jim, is there a way we can 'read' the windows 8 window border color? Or
> > maybe even have a chrome-only CSS flag/ pseudo-class called
> > `-moz-windowborder-dark` and `-moz-windowborder-light`, or something?
> 
> That's covered by bug 578780. We haven't had a lot of luck getting at the
> glass color in win7 but according to bug 578780, comment 22 there's a call
> we can make on Win8 to get the frame color.

Yes, but the caveat that Blair already applies there is found all through the internet. The value returned in WM_DWMCOLORIZATIONCOLORCHANGED is a lie (or at least alphamultiplied in a fashion that makes it Hard to figure out the real color). The best I could find was this: http://stackoverflow.com/a/22956468/713326 (untested).

Also, as comment #0 states, if we think this is chemspill-worthy, adding a CSS pseudoclass isn't really a workable option.

I think we should (1) change our Windows 8.1 detection or uplift 907373 to release, and (2) read the Balance registry key, too, and do some of the math so that we get the color we're computing right.
(to be clear, this is talking about beta/release upliftable solutions - having a proper API for this is better for 31/32, but Windows isn't particularly forthcoming with information...)
Gijs, any lead on this bug?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Jim Mathies [:jimm] from comment #4)
> > (In reply to Mike de Boer [:mikedeboer] from comment #3)
> > > Ehm, I can?
> > > 
> > > Jim, is there a way we can 'read' the windows 8 window border color? Or
> > > maybe even have a chrome-only CSS flag/ pseudo-class called
> > > `-moz-windowborder-dark` and `-moz-windowborder-light`, or something?
> > 
> > That's covered by bug 578780. We haven't had a lot of luck getting at the
> > glass color in win7 but according to bug 578780, comment 22 there's a call
> > we can make on Win8 to get the frame color.
> 
> Yes, but the caveat that Blair already applies there is found all through
> the internet. The value returned in WM_DWMCOLORIZATIONCOLORCHANGED is a lie
> (or at least alphamultiplied in a fashion that makes it Hard to figure out
> the real color). The best I could find was this:
> http://stackoverflow.com/a/22956468/713326 (untested).

Oh, too bad.. I thought we got a clean value in win8 from that api. That color munging code posted by patrick appears to work pretty well though. We could confirm that math is right by filing an ms support request if need be. (Note the turn around time on a support request would be a few weeks or longer.) Or we could just use it and see what happens.

> I think we should (1) change our Windows 8.1 detection or uplift 907373 to
> release

That uplift was denied so I guess we're stuck with busted tabs in Win8 on release. The detection bug 907373 is currently in Fx30 (beta).

So if we do a fix that involves css changes, we can land on mc, aurora, and maybe beta in the next 6 weeks. We'd need to get it out quickly though.
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Gijs, any lead on this bug?

No, I've not been working on this, although I had some discussions with Jared about how to approach it. At the last Australis meeting, Jared volunteered to drive this, so let's check with him.
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
I *think* it'd be fine to broadcast WM_DWMCOLORIZATIONCOLORCHANGED events in such a way that chrome JS can receive them and apply the necessary math there.

Once we've got that, we can set the `darkwindowframe` attribute as done in http://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1193.
(In reply to Mike de Boer [:mikedeboer] from comment #11)
> I *think* it'd be fine to broadcast WM_DWMCOLORIZATIONCOLORCHANGED events in
> such a way that chrome JS can receive them and apply the necessary math
> there.

Jim et al. are working on this in bug 578780. That's still not helpful for beta/release, as I said in comment 6.

> Once we've got that, we can set the `darkwindowframe` attribute as done in
> http://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> js#1193.

We should just try to update this math to get it right in more of the cases.
Flags: needinfo?(gijskruitbosch+bugs)
I tweaked the first two bytes of the ColorizationColor and ColorizationColorBalance values via the registry and only saw changes to the color when changing the ColorizationColorBalance value.

When ColorizationColorBalance is 0, the window frame color is a light-grey, even when the ColorizationColor value has a non-zero alpha value and the rgb components are (255,0,0).

Using the ColorizationColorBalance value as the alpha value for blending the foreground color (specified by ColorizationColor) and the background color (hardcoded to rgb(217,217,217)) produces the correct colors here.

I'm unable to reproduce a situation where the window frame is at-or-near black while ColorizationColor reports a non-black foreground color at the same time as ColorizationColorBalance specifies an opaque or near-opaque foreground color.
Assignee: nobody → jaws
Attachment #8417660 - Flags: review?(gijskruitbosch+bugs)
Flags: needinfo?(jaws)
Comment on attachment 8417660 [details] [diff] [review]
Patch (white text on light-grey window frame)

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

::: browser/modules/Windows8WindowFrameColor.jsm
@@ +17,5 @@
>    get: function() {
>      if (this._windowFrameColor)
>        return this._windowFrameColor;
>  
> +    let customizationColor = WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,

Nit: make the current user thing a local const to shorten this up a bit.

Nit: can probably also make the registry just import as "Registry", because it's clear that we're talking about the windows one.

@@ +29,5 @@
> +    let [unused, fgR, fgG, fgB] = customizationColorArray.map(function(val) parseInt(val, 16));
> +    let colorizationColorBalance = WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
> +                                                              "Software\\Microsoft\\Windows\\DWM",
> +                                                              "ColorizationColorBalance");
> +    let frameBaseColor = 217; // Window frame base color when Color Intensity is at 0

Maybe reference this bug?
Attachment #8417660 - Flags: review?(gijskruitbosch+bugs) → review+
Landed on fx-team,
https://hg.mozilla.org/integration/fx-team/rev/5ba1b3073051
Whiteboard: [fixed-in-fx-team]
Duplicate of this bug: 998231
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team][Australis:P2]
https://hg.mozilla.org/mozilla-central/rev/5ba1b3073051
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][Australis:P2] → [Australis:P2]
Target Milestone: --- → Firefox 32
Comment on attachment 8417660 [details] [diff] [review]
Patch (white text on light-grey window frame)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): incomplete implementation of fix in bug 940393
User impact if declined: people who have moved the Color Intensity slider all the way to the 0-level or close to it, may have white text on a light-grey background
Testing completed (on m-c, etc.): locally, and on m-c
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none
Attachment #8417660 - Flags: approval-mozilla-beta?
Attachment #8417660 - Flags: approval-mozilla-aurora?
Switching 29 to wontfix as we missed the cutoff for the 29.0.1 build.
Attachment #8417660 - Flags: approval-mozilla-beta?
Attachment #8417660 - Flags: approval-mozilla-beta+
Attachment #8417660 - Flags: approval-mozilla-aurora?
Attachment #8417660 - Flags: approval-mozilla-aurora+
Um, so this is actually much easier to reproduce than using sliders just by using built-in High-contrast themes in Windows 8. I totally ended up posting in the wrong bugs :(
Keywords: verifyme
Blocks: 1007229
(In reply to [:Cww] from comment #20)
> Um, so this is actually much easier to reproduce than using sliders just by
> using built-in High-contrast themes in Windows 8. I totally ended up posting
> in the wrong bugs :(

<insert choice expletives here>

I mistakenly assumed MS's APIs about this would be sane, because they pretend that these themes are "just like other themes" (see bug 946595). But they're really not, from the looks of it, because this is still broken on (a) high contrast theme(s). Filed bug 1007229.
(In reply to :Gijs Kruitbosch from comment #22)
> (In reply to [:Cww] from comment #20)
> > Um, so this is actually much easier to reproduce than using sliders just by
> > using built-in High-contrast themes in Windows 8. I totally ended up posting
> > in the wrong bugs :(
> 
> <insert choice expletives here>

(just to be clear: not directed at you. Thank you for pointing this out, because at least now we know this case is still broken.)
Component: Tabbed Browser → Theme
Reproduced the initial issue on Firefox 29.0RC, verified as fixed on Firefox 29.0.1 build 2, Firefox 30 beta 3, latest Aurora and latest Nightly.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #19)
> Switching 29 to wontfix as we missed the cutoff for the 29.0.1 build.

This was marked as fixed on 29 by Bogdan (see comment #25). As far as I can tell from https://tbpl.mozilla.org/?tree=Mozilla-Release, that is inaccurate, right?
Flags: needinfo?(jaws)
Yeah, seems inaccurate to me. Bogdan, can you re-test 29.0.1? It shouldn't be fixed there.
Flags: needinfo?(jaws) → needinfo?(bogdan.maris)
I retested on Firefox 29.0.1 build 1 and build 2 on PC and Surface Pro2 using the default Windows theme with black borders the background tabs are readable (image - http://goo.gl/YVj8bb).

Using a custom theme (Gray8.1 glass) only latest Nightly has readable background tabs, I think that the theme uses high contrast as well.
Flags: needinfo?(bogdan.maris)
Bogdan, the initial reported issue is slightly different than the issue that got fixed by the patch in this bug.

To reproduce this bug, you should select a dark color Windows theme, then in the Colors and Appearance dialog, slide the "Color intensity" slider all the way to the left, so that the window frame is a light grey. This should produce white text on a light background, which makes the background tabs hard to read.

Can you please re-verify?
I'm a bit caught up at the moment but I will follow up on this first thing tomorrow. Thanks!
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #29)
> Bogdan, the initial reported issue is slightly different than the issue that
> got fixed by the patch in this bug.
> 
> To reproduce this bug, you should select a dark color Windows theme, then in
> the Colors and Appearance dialog, slide the "Color intensity" slider all the
> way to the left, so that the window frame is a light grey. This should
> produce white text on a light background, which makes the background tabs
> hard to read.
> 
> Can you please re-verify?
I reproduced this on 4 different machines with and Windows 8.1 using Nightly from 2014-05-04. Did not reproduce the issue on Firefox 29.0. Text from background tabs is black, background is light so it`s easy to read exactly the same as latest Aurora and latest Nightly. (image - http://goo.gl/ATfK2p)

STR used:
1. Personalize
2. Colour and Appearance
3. Select 'Colour 1'
4. Click on Show color mixer and move Brightness to max left
5. Move 'Colour intensity' to max left
6. Save

Am I missing something here?
Flags: needinfo?(bogdan.maris) → needinfo?(jaws)
Those steps make sense to me. I'm not sure why your 32.0a1 shows black on grey, maybe it wasn't restarted? Did you ever see white text on your 29 build?
Flags: needinfo?(jaws)
Flags: needinfo?(bogdan.maris)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #32)
> Those steps make sense to me. I'm not sure why your 32.0a1 shows black on
> grey, maybe it wasn't restarted?
Nightly 32.0a1 from bottom right is the old nightly and only the tab that`s on focus has black text, it was restarted.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #32)
> Did you ever see white text on your 29 build?
No, every time I open Firefox 29 or 29.01 I get the same result: black text on grey background, just like in the image from comment 31. (I retested again on a different machine from the ones I reproduced earlier).
Flags: needinfo?(bogdan.maris)
Jared, can you follow up on comment 33?
Flags: needinfo?(jaws)
I just noticed in your comment #28 that you said you were using a custom theme. This bug is not related to custom themes, only changing the settings for Windows. Were you testing recently with the custom theme?
Flags: needinfo?(jaws) → needinfo?(bogdan.maris)
(In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo? me) from comment #35)
> I just noticed in your comment #28 that you said you were using a custom
> theme. This bug is not related to custom themes, only changing the settings
> for Windows. Were you testing recently with the custom theme?

No, my recent testing was done changing only the settings from Colour and Appearance.
Flags: needinfo?(bogdan.maris)
Keeping verifyme on this bug until the test results have been finalized.
Keywords: verifyme
Based on my reply is it enough to change the tracking flags back to verified and close this?
Flags: needinfo?(jaws)
Removing "verifyme" keyword since there doesn't seem to be any additional QA work needed, and it's a matter of confirming the test results. Will update the bug when Jared returns.
Keywords: verifyme
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #39)
> Removing "verifyme" keyword since there doesn't seem to be any additional QA
> work needed, and it's a matter of confirming the test results. Will update
> the bug when Jared returns.

Fair enough, lets just mark this verified until proven otherwise.
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #36)
> (In reply to Jared Wein [:jaws] [Away for most of July] (please needinfo?
> me) from comment #35)
> > I just noticed in your comment #28 that you said you were using a custom
> > theme. This bug is not related to custom themes, only changing the settings
> > for Windows. Were you testing recently with the custom theme?
> 
> No, my recent testing was done changing only the settings from Colour and
> Appearance.

Hm, I'm perplexed about this, but I haven't seen any bug reports come in about tabs being unreadable anymore. Given that, I think we can call this bug verified and stop any QA-verification efforts here. Thanks for the deep dive on this one!
Flags: needinfo?(jaws)
You need to log in before you can comment on or make changes to this bug.