Daniel Holbert [:dholbert] Reporter
	Description • 9 years ago

STR:
 1. Start Nightly with a fresh profile.
 2. Look at the notification bar that appears about e10s.

ACTUAL RESULTS: The bar is unreadable; it's got white text on a near-white background.

EXPECTED RESULTS: Readable text.


(In yesterday's nightly, pre-gtk3, it had white text on a black background.)

	Daniel Holbert [:dholbert] Reporter
	Comment 1 • 9 years ago

Hitting this bug in today's nightly, version 42.0a1 (2015-07-23)

	Daniel Holbert [:dholbert] Reporter
	Comment 2 • 9 years ago

I'm using Ubuntu 15.04 with gnome-shell and default theming, btw.

	Julien Wajsberg [:julienw]
	Comment 3 • 9 years ago

Here (Debian Stable) I have a white text on a black background. (Same nightly as you: BuildID is 20150723030207)

	Julien Wajsberg [:julienw]
	Comment 4 • 9 years ago

However the "close" button is not visible until I hover it.

	Daniel Holbert [:dholbert] Reporter
	Comment 5 • 9 years ago

Looks like the theming here comes from:
> 7  notification {
> 8    color: InfoText;
> 9    background-color: InfoBackground;

http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/global/notification.css?rev=4c1105d5b6a9#7

Here's a reduced testcase with that same styling, which shows the same issue for me -- whitish text on whitish background, in Nightly.

In Firefox Release, this renders with whitish text on a black background.

	Daniel Holbert [:dholbert] Reporter
	Comment 6 • 9 years ago

It looks like InfoBackground ends up mapping to "sInfoBackground", which is set to GTK's tooltip background color. And InfoText / sInfoText is supposed to be the tooltip foreground. That gets set up here:

> 1008     // tooltip foreground and background
> 1009     gtk_style_context_add_class(style, GTK_STYLE_CLASS_TOOLTIP);
> 1010     gtk_style_context_add_class(style, GTK_STYLE_CLASS_BACKGROUND);
> 1011     gtk_style_context_get_background_color(style, GTK_STATE_FLAG_NORMAL, &color);
> 1012     sInfoBackground = GDK_RGBA_TO_NS_RGBA(color);
> 1013     gtk_style_context_get_color(style, GTK_STATE_FLAG_NORMAL, &color);
> 1014     sInfoText = GDK_RGBA_TO_NS_RGBA(color);
http://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsLookAndFeel.cpp#1009

When I step through this in GDB, I can confirm that I'm getting the following near-white |color| for the background:
{
  red = 0.94901960784313721, 
  green = 0.94509803921568625, 
  blue = 0.94117647058823528, 
  alpha = 1
}

...and the following fully-white color for the foreground (text):
{
  red = 1, 
  green = 1, 
  blue = 1, 
  alpha = 1
}

So the problem is somewhere around here. Maybe we're querying for the tooltip background color incorrectly?

(I suppose it's possible my theme is just insane -- but gnome-tweak tool shows that my GTK+ theme is "Ambiance", and /usr/share/themes/Ambiance/gtk-3.0/gtk-main.css has:
> @define-color tooltip_bg_color #000000;
> @define-color tooltip_fg_color #ffffff;

...so assuming it applies those correctly, I'd expect that the tooltip background color should be 0,0,0 == black.)

	Daniel Holbert [:dholbert] Reporter
	Comment 7 • 9 years ago

OK, so it turns out this is partly my theme's fault.

My theme actually styles tooltip backgrounds like so:
.tooltip {
    background-image: -gtk-gradient (radial, center 0, 0, center 0, 0.8,
                                     from (alpha (mix (@tooltip_bg_color, #ffffff, 0.2), 0.86)),
                                     to (alpha (@tooltip_bg_color, 0.86)));

It *ONLY* sets background-image (to some gradient based on @tooltip_bg_color). It never sets background-color.  So, .tooltip ends up with some default .background-color provided by some other element. This doesn't matter for *actual* tooltip rendering, because the tooltips render with the dark background-image. But it does matter for Firefox, since we expect there to be a sane background-color and we use that color (instead of the background-image) for rendering notification info-bars.

	Daniel Holbert [:dholbert] Reporter
	Comment 8 • 9 years ago

(Comment 7's gtk CSS is from
 /usr/share/themes/Ambiance/gtk-3.0/gtk-widgets.css

If I add a "background-color: @tooltip_bg_color" to that .tooltip style rule in gtk-widgets.css, and then start up Firefox and view the attached testcase, then my attached testcase renders with a black InfoBackground. (And if I use "red" instead of @tooltip_bg_color, then it renders with a red InfoBackground.)

SO:
 - one fix here, on the GTK side, is for gtk-widgets.css to use "background-color: @tooltip_bg_color. I'll see if I can find a place to file that upstream.
 - Separately: should we be worried about other GTK themes doing similar things? If it's common for themes to use "background-image" on tooltips, then it might also be common for them to disregard "background-color", which means we can't reliably depend on it to be useful. :-/

	Daniel Holbert [:dholbert] Reporter
	Comment 11 • 9 years ago

Two more notes here:
 - This affects the GTK3 "Radiance" theme as well (which is shipped with "Ambiance" and has the same developer -- I notified them about that theme as well).

 - This also affects e.g. NoScript's notification bars which appear at the bottom of Firefox windows to tell you that scripts have been blocked.  (Probably many more types of notification bars, as well. This NoScript notification & the new-profile multiprocess notification are just easy to summon on-demand.)

	Daniel Holbert [:dholbert] Reporter
	Comment 12 • 9 years ago

(In reply to Daniel Holbert [:dholbert] from comment #9)
> Looks like the Ambiance theme is maintained by RAVEfinity:

Sorry, this wasn't quite right actually -- RaveFinity (& Jared) develop a derivative theme called "Ambiance Colors", not the base "Ambiance" theme itself.

It looks like the Ambiance & Radiance themes are maintained by Ubuntu folks, and they're part of this package: https://launchpad.net/light-themes

	Daniel Holbert [:dholbert] Reporter
	Comment 13 • 9 years ago

Filed bug upstream at Ubuntu:
 https://bugs.launchpad.net/ubuntu/+source/ubuntu-themes/+bug/1478173

	Daniel Holbert [:dholbert] Reporter
	Comment 14 • 9 years ago

This bug also breaks Firefox's RSS Feed UI (white-text-on-nearly-white-background).

The same fix (described in comment 8) fixes this rendering, too -- the top section ends up getting a black background, and the text becomes readable.

	Daniel Holbert [:dholbert] Reporter
	Comment 15 • 9 years ago

This is the patch I submitted upstream at Launchpad. I can fix this bug locally by doing...
 cd /usr/share/themes/
 sudo patch -p1 < /tmp/theme-patch.patch
...and restarting Firefox.

	Karl Tomlinson (:karlt)
	Comment 16 • 9 years ago

(In reply to Daniel Holbert [:dholbert] from comment #8)
>  - Separately: should we be worried about other GTK themes doing similar
> things? If it's common for themes to use "background-image" on tooltips,
> then it might also be common for them to disregard "background-color", which
> means we can't reliably depend on it to be useful. :-/

Yes, I suspect this may be common.  There have been similar problems with borders.
-moz-appearance:window/NS_THEME_WINDOW and probably some extras may be easier than trying to map all of GTK CSS to Gecko.

Sometimes though UI wants "matching" colors for other elements, which are not going to be drawn by GTK.  I don't know whether there is a always a -moz-appearance kind of solution there or whether it is necessary to resort to sampling colors from GTK drawing.

	Dão Gottwald [:dao]
	Comment 18 • 9 years ago

[Tracking Requested - why for this release]:

This affects all sorts of notification bars, and not only notifications bars, because infotext and infobackground are used in other contexts too.

	Kate Glazko
	Comment 19 • 9 years ago

Do you know what the regressing bug is? Sounds like a U.I. regression since it isn't affected in 41.

	Kate Glazko
	Comment 20 • 9 years ago

Tracking, see Comment 18.

	Daniel Holbert [:dholbert] Reporter
	Comment 21 • 9 years ago

It's a regression from switching GTK3 support on in our builds. (I forget precisely which bug made the switch, but this is already marked as blocking the GTK3 metabug.)

It is a UI regression, but it's caused by us querying a different piece of the underlying OS for theming information (and that different piece providing different answers due to representing the theme in a different way).

	Daniel Holbert [:dholbert] Reporter
	Comment 22 • 9 years ago

(In reply to Daniel Holbert [:dholbert] from comment #21)
> It's a regression from switching GTK3 support on in our builds. (I forget
> precisely which bug made the switch

Looks like the proximal cause (the commit that enabled the feature) was bug 1186003.  Adding that to "blocking" field as well, for better tracking of exactly when this broke.

	Andrew Comminos [:acomminos] Assignee
	Comment 26 • 9 years ago

This patch adds a -moz-appearance style for GtkInfoBar, a good candidate for providing styling where we would previously use infotext and infobackground. I think using this on GTK3 is cleaner than working around incomplete theme styles- we should avoid nsLookAndFeel for background styling in general if we are to follow the GTK3 api's advice of not using the theme's background-color properties.

Thanks!

	Karl Tomlinson (:karlt)
	Comment 30 • 9 years ago

Comment on attachment 8650066 [details] [diff] [review]
Use -moz-gtk-info-bar for RSS feed CSS on Linux.

> #feedHeaderContainer {
>   border: 1px solid ThreeDShadow;
>   border-radius: 10px;
>+  color: InfoText;
>   margin: -4em auto 0 auto;
>   background-color: InfoBackground;
>+  -moz-appearance: -moz-gtk-info-bar;
> }
> 
> #feedHeader {  
>   margin-top: 4.9em;
>   margin-bottom: 1em;
>   -moz-margin-start: 1.4em;
>   -moz-margin-end: 1em;
>   -moz-padding-start: 2.9em;
>   font-size: 110%;
>   color: InfoText;
> }

Please remove the new "color: InfoText;" as #feedHeader provides this.
The rest is good thanks.

	Karl Tomlinson (:karlt)
	Comment 31 • 9 years ago

Comment on attachment 8650064 [details] [diff] [review]
Implement GtkInfoBar appearance style on GTK3.

>-    // tooltip foreground and background
>-    gtk_style_context_add_class(style, GTK_STYLE_CLASS_TOOLTIP);
>+    // info bar text and background
>     gtk_style_context_add_class(style, GTK_STYLE_CLASS_BACKGROUND);

>+    gtk_style_context_add_class(style, GTK_STYLE_CLASS_INFO);

GtkInfoBar looks good for notifications, but InfoText is actually intended and
used for tooltips:

https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#System_Colors
http://hg.mozilla.org/mozilla-central/annotate/d590b9601ba8/toolkit/themes/linux/global/popup.css#l91

If the backgrounds of tooltips and notifications are drawn differently, which
seems preferable, then we need a new color to support the differnent
foreground colors.

The style context doesn't match what GtkInfoBar uses.
The create_context() code gets closer, but it seems it is hard to get the same
context without actually creating widgets, so creating a GtkInfoBar is
probably best.

If you'd prefer to go with an interim solution, then perhaps
-moz-appearance: tooltip could be used until there is a foreground color to
match -moz-gtk-info-bar.

>+static gint
>+ensure_info_bar()
>+{
>+  if (!gInfoBar) {
>+      gInfoBar = gtk_info_bar_new();
>+      setup_widget_prototype(gInfoBar);
>+  }
>+  return MOZ_GTK_SUCCESS;

Just return void because this never fails and the return value is never
checked.  I know the other methods do this, but there's no need to follow.

	Andrew Comminos [:acomminos] Assignee
	Comment 32 • 9 years ago

This adds -moz-gtk-info-bar-text, falling back to InfoText on GTK2. Style contexts should be properly set up now as well.

Thanks!

	Andrew Comminos [:acomminos] Assignee
	Comment 33 • 9 years ago

Update to use -moz-gtk-info-bar-text.

	Andrew Comminos [:acomminos] Assignee
	Comment 34 • 9 years ago

Updated to address review comments and use -moz-gtk-info-bar-text.

	Karl Tomlinson (:karlt)
	Comment 35 • 9 years ago

Comment on attachment 8650533 [details] [diff] [review]
Implement GtkInfoBar appearance style on GTK3.

Looks great, thanks.

>   eCSSKeyword__moz_mac_vibrancy_light,        NS_THEME_MAC_VIBRANCY_LIGHT,
>   eCSSKeyword__moz_mac_vibrancy_dark,         NS_THEME_MAC_VIBRANCY_DARK,
>   eCSSKeyword__moz_mac_disclosure_button_open,   NS_THEME_MAC_DISCLOSURE_BUTTON_OPEN,
>   eCSSKeyword__moz_mac_disclosure_button_closed, NS_THEME_MAC_DISCLOSURE_BUTTON_CLOSED,
>+  eCSSKeyword__moz_gtk_info_bar, NS_THEME_GTK_INFO_BAR,

Should probably pick something to align NS_THEME_GTK_INFO_BAR with.

I would pick NS_THEME_MAC_VIBRANCY_DARK, as NS_THEME_MAC_DISCLOSURE_BUTTON_*
seem to be exceptions.


>   eCSSKeyword__moz_oddtreerow, LookAndFeel::eColorID__moz_oddtreerow,
>   eCSSKeyword__moz_visitedhyperlinktext, NS_COLOR_MOZ_VISITEDHYPERLINKTEXT,
>   eCSSKeyword_currentcolor, NS_COLOR_CURRENTCOLOR,
>   eCSSKeyword__moz_win_mediatext, LookAndFeel::eColorID__moz_win_mediatext,
>   eCSSKeyword__moz_win_communicationstext, LookAndFeel::eColorID__moz_win_communicationstext,
>   eCSSKeyword__moz_nativehyperlinktext, LookAndFeel::eColorID__moz_nativehyperlinktext,
>   eCSSKeyword__moz_comboboxtext, LookAndFeel::eColorID__moz_comboboxtext,
>   eCSSKeyword__moz_combobox, LookAndFeel::eColorID__moz_combobox,
>+  eCSSKeyword__moz_gtk_info_bar_text, LookAndFeel::eColorID__moz_gtk_info_bar_text,

With some exceptions at the end, these were almost in alphabetical order, so
please insert before eCSSKeyword__moz_hyperlinktext.

	Andrew Comminos [:acomminos] Assignee
	Comment 36 • 9 years ago

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d171945f00db

	Pulsebot
	Comment 37 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7591ecf5987
https://hg.mozilla.org/integration/mozilla-inbound/rev/6194cebe2a84
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea072f95e28e

	Andrew Comminos [:acomminos] Assignee
	Comment 38 • 9 years ago

Updated for review comments, was in the last push.

	Ryan VanderMeulen [:RyanVM]
	Comment 39 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/e7591ecf5987
https://hg.mozilla.org/mozilla-central/rev/6194cebe2a84
https://hg.mozilla.org/mozilla-central/rev/ea072f95e28e

	Sylvestre Ledru [:Sylvestre]
	Comment 42 • 9 years ago

Andrew, could you fill the uplift request to aurora/42? Thanks

	Andrew Comminos [:acomminos] Assignee
	Comment 43 • 9 years ago

Comment on attachment 8651227 [details] [diff] [review]
Implement GtkInfoBar appearance style on GTK3. Carry r=karlt

Sure. Here's a request for all three patches to be included;

Approval Request Comment
[Feature/regressing bug #]: Bug was introduced when gtk3 was turned on, fixed in bug 1187203.
[User impact if declined]: Notification bars will be hard to read on some GTK3 themes (e.g. Ubuntu's Ambiance).
[Describe test coverage new/current, TreeHerder]: N/A
[Risks and why]: No outstanding risks.
[String/UUID change made/needed]: None.

	Sylvestre Ledru [:Sylvestre]
	Comment 44 • 9 years ago

Comment on attachment 8651227 [details] [diff] [review]
Implement GtkInfoBar appearance style on GTK3. Carry r=karlt

Taking it to improve the support of gtk3.

	Ryan VanderMeulen [:RyanVM]
	Comment 45 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/f027a14fdf66
https://hg.mozilla.org/releases/mozilla-aurora/rev/cd9769e27378
https://hg.mozilla.org/releases/mozilla-aurora/rev/20c7ffc5f121

	uoggegloo
	Comment 46 • 8 years ago

Andrew, `-moz-appearance: -moz-gtk-info-bar` messes up the borders for feedHeaderContainer in the RSS preview interface ...

In the theme I use ("TraditionalOk" in Mate Desktop), the side borders are missing, the bottom border color doesn't match the border color for feedBody, and the border-radius declaration isn't respected.

Interestingly, Inspector shows the missing styles as active, however, I can only see them when I disable `-moz-appearance: -moz-gtk-info-bar`

Should I open a new bug?