1287036 - GTK3 menuitem non-hover background property ignored

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P4)

Description

[Eric]

Attachment: gtk3_nonhover_menu_items_background_image_property_not_respected.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.9) Gecko/20100101 Goanna/2.1 Firefox/38.9 PaleMoon/26.3.3
Build ID: 20160629210805

Steps to reproduce:

In gtk-widgets.css for my theme, I set the following code:

.menu .menuitem{
	background-image: url("assets/menubar_luna.png");
	background-repeat: repeat-y;
	padding-left: 8px;
}

With this rule, menuitems should respect the background-image setting.  However, in Firefox 47.0, they do not. 


Actual results:

Firefox does not respect the menuitems rule which affects menu items listed under the top most menu dropdown.  


Expected results:

Firefox renders the background-image on each menuitem.  When hovering over the menu item (.menu .menuitem:hover) the background-image rule is respected, but not for non-hovered menu items.

The attached image shows what each menuitem should look like when properly respected the rule like gedit does for the GTK3 theme.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: bug 1287036 draw menuitem background even when not in hover

Review commit: https://reviewboard.mozilla.org/r/91306/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91306/

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: bug 1287036 draw separator menuitem background and frame behind separator in menu

as does gtk_menu_item_draw().

Review commit: https://reviewboard.mozilla.org/r/91308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91308/

Comment 3

[Martin Stránský [:stransky] (ni? me)]

I tested the patch and works fine when menuitem background is not animated.

But it brings an interesting regression when menuitem background is animated (css transition), like Menta theme uses...I still investigate where the bug comes from.

Comment 4

[Karl Tomlinson (:karlt)]

Thanks, Martin.  The animation issue may be related to bug 1315668.
I have some suspicions, which I'll investigate there.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Attachment: animated background - menu is active

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Attachment: animated bacground - menu inactive

Comment 7

[Martin Stránský [:stransky] (ni? me)]

I think those two pictures demonstrates what happens when the background is animated and also inactive items are painted. The style changes in time according to animation but we draw the menu items sequentially - so each menuitem got a bit different style in time according to the color fade.

The issue it already here but it's not so visible when we draw only the the active items.

I wonder if it's possible to recognize the animated styles somehow or we may use unique styles with different states for active/inactive items...

Comment 8

[Martin Stránský [:stransky] (ni? me)]

(In reply to Karl Tomlinson (:karlt) from comment #4)
> Thanks, Martin.  The animation issue may be related to bug 1315668.
> I have some suspicions, which I'll investigate there.

Yes I see that too. The pictures above comes from FF49 where the animated background it still working.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8808480 [details]
bug 1287036 draw menuitem background even when not in hover

https://reviewboard.mozilla.org/r/91306/#review93426

We're fine here with Bug 1315668 fixed.

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

https://reviewboard.mozilla.org/r/91308/#review93428

Okay.

Comment 11

[Karl Tomlinson (:karlt)]

Comment on attachment 8808480 [details]
bug 1287036 draw menuitem background even when not in hover

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91306/diff/1-2/

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91308/diff/1-2/

Comment 13

[Karl Tomlinson (:karlt)]

Comment on attachment 8808480 [details]
bug 1287036 draw menuitem background even when not in hover

https://reviewboard.mozilla.org/r/91304/#review93690

Revision 2 is a rebase on top of changes for bug 1315668.

Comment 14

[Karl Tomlinson (:karlt)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

https://reviewboard.mozilla.org/r/91308/#review93428

Thanks, Martin.  I since realized that this should use the context from a GtkSeparatorMenuItem, rather than a plain GtkMenuItem, in case the theme renders them differently, which happens in GTK 3.20.  See bug 1303964.
I request review again to check you are happy with that change.

Comment 15

[Karl Tomlinson (:karlt)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91308/diff/2-3/

Comment 16

[Karl Tomlinson (:karlt)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

Requesting review again for MOZ_GTK_MENUSEPARATOR.
Sorry that I missed that before.

Comment 17

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8808481 [details]
bug 1287036 draw separator menuitem background and frame behind separator in menu

https://reviewboard.mozilla.org/r/91308/#review94160

Good!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe6045ca3c46
draw menuitem background even when not in hover r=stransky+263117
https://hg.mozilla.org/integration/autoland/rev/7a0ad5f1a7f6
draw separator menuitem background and frame behind separator in menu r=stransky+263117

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fe6045ca3c46
https://hg.mozilla.org/mozilla-central/rev/7a0ad5f1a7f6