Closed Bug 1208616 Opened 9 years ago Closed 4 years ago

[Windows 7 aero glass][Light theme] White text in Menu bar and TabsToolbar is barely visible on light background

Categories

(Firefox :: Theme, defect, P2)

Product:
Firefox
Component:
Theme
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
firefox44 --- wontfix
firefox53 + wontfix
firefox54 + fix-optional
firefox55 --- affected

People

(Reporter: arni2033, Unassigned)

References

Details

(Whiteboard: [btpp-fix-later])

Attachments

(10 files, 5 obsolete files)

screenshot - [Dev Theme] White text in Menu bar and TabsToolbar is barely visible on light background.png
95.20 KB, image/png
Details
video - [Dev Theme] White text in Menu bar and TabsToolbar is barely visible on light background.webm
129.69 KB, video/webm
Details
Dark Theme - Win 7 Aero - Menu Bar.png
225.90 KB, image/png
Details
Default Theme - Win 7 Aero - Menu Bar.png
226.97 KB, image/png
Details
Light Theme - Win 7 Aero - Menu Bar.png
120.56 KB, image/png
Details
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.
59 bytes, text/x-review-board-request
Details
Attempt 3 - Light Theme.png
127.97 KB, image/png
Details
Attempt 3 - Light Theme (Inactive).png
93.54 KB, image/png
Details
Attempt 3 - Dark Theme.png
121.91 KB, image/png
Details
Attempt 3 - Dark Theme (Inactive).png
122.96 KB, image/png
Details 
STR:   (Win7_64, Nightly 44, 32bit, ID 20150924030231, new profile, safe mode)
1. Open menu (≡) -> click "Customize"
2. Set "Developer Edition" theme, enable bookmarks toolbar, enable menu bar
3. Place bookmark items and zoom level widget to the TabsToolbar, exit customize
4. Resize window to normal mode

Result:       All those items display white-on-white text. Actually white icons aren't great as well
Expectations: Either (A) or (B)
           A) (preferable) menu bar and TabsToolbar have black background, just like in maximized mode
           B) Text color should be black in TabsToolbar and menu bar on Windows
Has STR: --- → yes
Priority: -- → P2
Whiteboard: [btpp-fix-now]
Whiteboard: [btpp-fix-now] → [devedition-polish][btpp-fix-later]
Blocks: 1331679
I've added screenshots of Nightly 54 with Windows 7 Aero theme configured with menu bar enabled and widgets next to tabs like the reporter.

The menu bar / widgets seem more readable in my own Compact Light screenshot than the original reporter's, but if you had a really light background, the contrast would definitely go down.  Comparing it with the Default theme, it seems the Default theme uses a stronger glass / fog effect on the window plus black text which is a bit easier to read, but I am not sure if it's desired for Compact themes.

The reporter purposed making the menu bar / tabs toolbar fully black, but that seems to make the tabs kind of disappear in my opinion.  It is confusing to me that we do essentially this in maximized mode though...

Gijs, any thoughts?  I am not sure if there is a change that would be clearly better here.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> I've added screenshots of Nightly 54 with Windows 7 Aero theme configured
> with menu bar enabled and widgets next to tabs like the reporter.
> 
> The menu bar / widgets seem more readable in my own Compact Light screenshot
> than the original reporter's, but if you had a really light background, the
> contrast would definitely go down.  Comparing it with the Default theme, it
> seems the Default theme uses a stronger glass / fog effect on the window
> plus black text which is a bit easier to read, but I am not sure if it's
> desired for Compact themes.

Yeah... if you use compact light, the text is black and that's really not a lot better when the glass is over a dark area (like in your screenshot) than the screenshots from the reporter of glass-over-white-area . So I think fog is likely the way to go here to get a consistent background for text. I assume we're disabling the fog somewhere, I bet just getting rid of that rule would help. Plus making sure that tabstrip text outside of the tabs is dark (not sure if that's going to be tricky or not).

> The reporter purposed making the menu bar / tabs toolbar fully black, but
> that seems to make the tabs kind of disappear in my opinion.  It is
> confusing to me that we do essentially this in maximized mode though...

This would be another option but I think the reason we don't do it right now is that it looks ugly in normal mode on anything before win10 because we can't make *everything* dark/white. So the window edges would still be aero-ish. But I'm not sure. Stephen would hopefully know...
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(shorlander)
(In reply to :Gijs from comment #5)
> > The reporter purposed making the menu bar / tabs toolbar fully black, but
> > that seems to make the tabs kind of disappear in my opinion.  It is
> > confusing to me that we do essentially this in maximized mode though...
> 
> This would be another option but I think the reason we don't do it right now
> is that it looks ugly in normal mode on anything before win10 because we
> can't make *everything* dark/white. So the window edges would still be
> aero-ish. But I'm not sure. Stephen would hopefully know...

Yeah we tried that initially but because of the constraints on where we can draw it mostly just ended up looking weird.

You could try adding a shadow to text on items outside of the tab strip similar to the window title or the default menu bar.
Flags: needinfo?(shorlander)
Tracking this since this was mentioned as a p2 issue for compact themes, which we plan to ship in 53. 
Gijs, is this something you'd be willing to take on, or do you know who might work on it?
status-firefox53: --- → affected
status-firefox54: --- → affected
tracking-firefox53: --- → +
tracking-firefox54: --- → +
Flags: needinfo?(gijskruitbosch+bugs)
I can do the work here, thought it seems like some fiddling will be need, since it's not obvious what would be best.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #8)
> I can do the work here, thought it seems like some fiddling will be need,
> since it's not obvious what would be best.

Yeah... I would still recommend trying to not disable the fog which we use in the default theme to solve this problem. Not sure where we're removing the fog right now, and if we're just not using the fog for lwthemes or something (on the assumption that those will have an opaque background, which the compact theme doesn't). It won't look amazing, but it will be usable, so that's a good start...
Comment on attachment 8835150 [details]
Dark Theme - Win 7 Aero - Menu Bar.png

I guess I confused things a bit here, this screenshot is from the Compact Dark theme.
Attachment #8835150 - Attachment description: Light Theme - Win 7 Aero - Menu Bar.png → Dark Theme - Win 7 Aero - Menu Bar.png
Attachment #8835150 - Attachment filename: Light Theme - Win 7 Aero - Menu Bar.png → Dark Theme - Win 7 Aero - Menu Bar.png
For a first attempt, I've made the following changes:

* Compact themes now apply the same glass fog effect as the Default theme
* Main menubar uses the rounded background from Default theme when the window is focused
* Main menubar always uses Compact Light's font color (#18191a) for both Light and Dark themes (since they are both using the same background color)
* No explicit changes to widgets in the tab toolbar, but they do get the fog effect behind them

Compact Dark screenshot: attachment 8840060 [details]
Compact Light screenshot: attachment 8840062 [details]
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

https://reviewboard.mozilla.org/r/114606/#review116654

I'm away today, but I'll look at this in more detail on Monday. From a cursory glance at the screenshots I did notice the text color in the tabstrip for the dark theme was still white with the fog being white, too, which didn't seem like it worked well.

Out of curiosity, would using dark fog for the dark theme work?

::: browser/themes/windows/compacttheme.css:82
(Diff revision 1)
> +
> +  /* When glass is used, always use the Compact Light font color (for both Light
> +     and Dark themes) for the main menu bar, which appears on top of the same
> +     rounded background from the Default theme when the window is active. */
> +  #main-menubar {
> +    color: #18191a;

From the screenshots it feels like we should do this for non-tab-items in the tabstrip as well?
Attachment #8840065 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

https://reviewboard.mozilla.org/r/114606/#review116656

Sorry, meant to leave this at r?.
Attachment #8840065 - Flags: review+ → review?(gijskruitbosch+bugs)
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

https://reviewboard.mozilla.org/r/114606/#review116654

Yes, I think dark fog does help!  It seems like it makes the background on window active part unnecessary too.  I'll post a new attempt with these changes.

> From the screenshots it feels like we should do this for non-tab-items in the tabstrip as well?

I considered doing this, but since there isn't a single container for all these items currently, it means each widget gets its own rounded background, which looks a bit silly to me.

I'll post a new attempt with the dark fog you suggested.  In my opinion, we don't need this background portion anymore with the dark fog.
Okay, for attempt 2, I've changed to a dark fog for Compact Dark and the menu bar background has been removed from both.

* Compact themes now apply the same glass fog effect as the Default theme
* Compact Light theme uses same fog color as Default theme
* Compact Dark theme uses the inverse fog color
* No explicit changes to widgets in the tab toolbar, but they do get the fog effect behind them

Compact Dark screenshot: attachment 8841085 [details]
Compact Light screenshot: attachment 8841086 [details]
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

Stephen, can you check this looks OK to you?
Attachment #8840065 - Flags: ui-review?(shorlander)
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

https://reviewboard.mozilla.org/r/114606/#review117066

I'm assuming the code is effectively copied over, so rs=me for that. The screenshots look OK to me, let's check with Stephen if he agrees. :-)
Attachment #8840065 - Flags: review?(gijskruitbosch+bugs) → review+
Stephen, please see the ui-review request (comment 27).  This is the last Compact Theme P2 bug on hand, so it would be nice to wrap this up, assuming it looks good to you.  Thanks!
Flags: needinfo?(shorlander)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #29)
> Stephen, please see the ui-review request (comment 27).  This is the last
> Compact Theme P2 bug on hand, so it would be nice to wrap this up, assuming
> it looks good to you.  Thanks!

Hi, question: Does this add the fog all the time even if you don't have menus shown or textual items in the tab strip?
Flags: needinfo?(shorlander) → needinfo?(jryans)
(In reply to Stephen Horlander [:shorlander] from comment #30)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #29)
> > Stephen, please see the ui-review request (comment 27).  This is the last
> > Compact Theme P2 bug on hand, so it would be nice to wrap this up, assuming
> > it looks good to you.  Thanks!
> 
> Hi, question: Does this add the fog all the time even if you don't have
> menus shown or textual items in the tab strip?

Yes, with the current changes, the fog would always be enabled for Compact Theme.

Is that what you are hoping for, or did you want it to appear only when those UI elements are used?
Flags: needinfo?(jryans)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #31)
> (In reply to Stephen Horlander [:shorlander] from comment #30)
> > (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #29)
> > > Stephen, please see the ui-review request (comment 27).  This is the last
> > > Compact Theme P2 bug on hand, so it would be nice to wrap this up, assuming
> > > it looks good to you.  Thanks!
> > 
> > Hi, question: Does this add the fog all the time even if you don't have
> > menus shown or textual items in the tab strip?
> 
> Yes, with the current changes, the fog would always be enabled for Compact
> Theme.
> 
> Is that what you are hoping for, or did you want it to appear only when
> those UI elements are used?

I don't think we should go with the fog approach. It was designed for Australis and doesn't really work that well with the flatter Dev Tools design.

I think we should keep the background on the menu bar, add a dark variant and then use text-shadow for items that could potentially be placed on the tab strip so they remain readable in more situations.

e.g. https://cl.ly/273a253y1o0r
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

Here's attempt 3, which tries to implement :shorlander's proposal in comment 32.

* Background added to menu bar when window is active
* Text shadow added to widgets placed next to tabs on the glass

Compact Light: attachment 8849229 [details]
Compact Light (inactive): attachment 8849231 [details]
Compact Dark: attachment 8849234 [details]
Compact Dark (inactive): attachment 8849235 [details]
Attachment #8840065 - Flags: ui-review?(shorlander)
Attachment #8840065 - Flags: review?(gijskruitbosch+bugs)
Attachment #8840065 - Flags: review+
(I'm going to delay reviewing until Stephen gives you ui-r+ :-) )
Comment on attachment 8840065 [details]
Bug 1208616 - Improve Compact Theme toolbars with Win 7 Glass.

https://reviewboard.mozilla.org/r/114606/#review124666

Based on the screenshot I would expect we'd want to use the inverted (ie white) icons on the menubar/tabstrip for the light theme on restored windows. The existing icons aren't really visible at all. :-(
The inverted icons also having black borders should ensure that they're visible even on bright background behind the glass, I think?

I'm also wondering if this code needs to be specific to restored windows? The selectors don't seem to care about this right now... Plus I'm wondering if we're sure the -moz-windows-glass media query doesn't apply on Win8/Win10.
Attachment #8840065 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs from comment #41)
> Based on the screenshot I would expect we'd want to use the inverted (ie
> white) icons on the menubar/tabstrip for the light theme on restored
> windows. The existing icons aren't really visible at all. :-(
> The inverted icons also having black borders should ensure that they're
> visible even on bright background behind the glass, I think?

What are "restored windows" here?  Is that like active vs. inactive / focused vs. unfocused?  Or something about session restore...?

> I'm also wondering if this code needs to be specific to restored windows?
> The selectors don't seem to care about this right now... Plus I'm wondering
> if we're sure the -moz-windows-glass media query doesn't apply on Win8/Win10.

Looks like -moz-windows-glass means "Glass and not Win 8 or later"[1], so it seems like it's filtering to Win 7 Glass only appropriately.

[1]: http://searchfox.org/mozilla-central/rev/0079c7adf3b329bff579d3bbe6ac7ba2f6218a19/widget/windows/nsLookAndFeel.cpp#417-420
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #42)
> (In reply to :Gijs from comment #41)
> > Based on the screenshot I would expect we'd want to use the inverted (ie
> > white) icons on the menubar/tabstrip for the light theme on restored
> > windows. The existing icons aren't really visible at all. :-(
> > The inverted icons also having black borders should ensure that they're
> > visible even on bright background behind the glass, I think?
> 
> What are "restored windows" here?  Is that like active vs. inactive /
> focused vs. unfocused?  Or something about session restore...?

"restored" is a window mode on Windows, also known (even more confusingly, IMO) as "normal". As opposed to maximized, minimized or fullscreen. In other words, what I'm saying is that maybe we don't want these styles to apply for maximized windows, because IIRC we do different things with the background color of the toolbox in maximized mode (and maybe fullscreen mode, too).
Flags: needinfo?(gijskruitbosch+bugs)
I haven't been able to get back to this work with other high priority bugs in the mix.  :canuckistani, how critical is it for this to be resolved in 53 (the first release with Compact Themes)?
Assignee: jryans → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(jgriffiths)
It's not critical. P1 bugs block, P2 don't.
Flags: needinfo?(jgriffiths)
Wontfix for 53, probably fix-optional for 54 though do request uplift if you come up with something good here.
status-firefox53: affected → wontfix
status-firefox54: affected → fix-optional
status-firefox55: --- → affected
Summary: [Dev Theme] White text in Menu bar and TabsToolbar is barely visible on light background → [Windows 7 aero glass][Light theme] White text in Menu bar and TabsToolbar is barely visible on light background
Whiteboard: [devedition-polish][btpp-fix-later] → [btpp-fix-later]
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: