Hussam Al-Tayeb Reporter
	Description • 8 years ago

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160217134259

Steps to reproduce:

The Menu bar is not colored (themed) correctly under gtk+ 3.18 (see attached screenshots)

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 3 • 8 years ago

That's caused by gtk_style_context_save(), see https://bugzilla.gnome.org/show_bug.cgi?id=761870 for details. Actually gtk_style_context_save() should not be used.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 4 • 8 years ago

There's the patch for it. The nsLookAndFeel.cpp part looks a bit untidy but we should update the whole code for the widget cache anyway.

Also, from my testing (on gtk 3.20) we don't need to set GTK_STYLE_CLASS_MENUITEM/GTK_STYLE_CLASS_MENUBAR styles - but I kept it here for compatibility reasons.

	Karl Tomlinson (:karlt)
	Comment 6 • 8 years ago

Comment on attachment 8749153 [details] [diff] [review]
patch

Thanks, Martin.  This is two separate issues, so please use two separate
patches and use descriptive commit messages for each.

>Bug 1248974 - render menu bar with correct style/color, r=?karlt

In the commit message, please state what code change is made.
i.e. indicate the change that makes this more correct now.

Something like "use GtkMenuItem child for menubar text color"
and "remove gtk_style_context_save that corrupts css node hierarchy for menu
item background".

Many of these widgets in LookAndFeel should have labels for the text colors
because GTK uses labels for text and some themes style the foreground color on
the label only, but that can be a separate issue.

>-        gtk_style_context_add_class(style, GTK_STYLE_CLASS_MENUITEM);
>+        bool hasMenuItemClass = gtk_style_context_has_class(style, GTK_STYLE_CLASS_MENUITEM);
>+        if (!hasMenuItemClass) {
>+            gtk_style_context_add_class(style, GTK_STYLE_CLASS_MENUITEM);
>+        }

GTK versions up to 3.18.9 already add this in gtk_menu_item_init(), so this
will do nothing for them.

GTK 3.20 doesn't already have this, but it doesn't add it on drawing either,
and so this should not be added here.

Simply remove the unnecessary gtk_style_context_add_class(style,
GTK_STYLE_CLASS_MENUITEM).  Then there is no need for the
gtk_style_context_remove_class(style, GTK_STYLE_CLASS_MENUITEM).

r+ with those changes.

(In reply to Martin Stránský from comment #4)
> There's the patch for it. The nsLookAndFeel.cpp part looks a bit untidy but
> we should update the whole code for the widget cache anyway.
> 
> Also, from my testing (on gtk 3.20) we don't need to set
> GTK_STYLE_CLASS_MENUITEM/GTK_STYLE_CLASS_MENUBAR styles - but I kept it here
> for compatibility reasons.

CLASS_MENUITEM is not required, as noted above.  CLASS_MENUBAR was in 3.4 but not 3.6.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 9 • 8 years ago

Thanks! There's a try build for those two patches: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb170068c41e

	Pulsebot
	Comment 10 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7c7214afc0d
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3c6d9772d8d

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b7c7214afc0d
https://hg.mozilla.org/mozilla-central/rev/a3c6d9772d8d