Closed Bug 1287036 Opened 8 years ago Closed 8 years ago

GTK3 menuitem non-hover background property ignored

Categories

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

Product:
Core
Component:
Widget: Gtk
Version:
47 Branch
Platform:
All
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: earnolmartin, Assigned: karlt)

References

Details

(Whiteboard: tpi:+)

Attachments

(5 files)

gtk3_nonhover_menu_items_background_image_property_not_respected.png
191.34 KB, image/png
Details
bug 1287036 draw menuitem background even when not in hover
58 bytes, text/x-review-board-request
stransky
: review+
Details
bug 1287036 draw separator menuitem background and frame behind separator in menu
58 bytes, text/x-review-board-request
stransky
: review+
Details
animated background - menu is active
62.06 KB, image/png
Details
animated bacground - menu inactive
68.28 KB, image/png
Details 
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.
OS: Unspecified → Linux
Hardware: Unspecified → All
Blocks: gtk3
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Priority: -- → P4
Whiteboard: tpi:+
Depends on: 1315873
Attachment #8808480 - Flags: review?(stransky)
Attachment #8808481 - Flags: review?(stransky)
Assignee: nobody → karlt
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.
Thanks, Martin.  The animation issue may be related to bug 1315668.
I have some suspicions, which I'll investigate there.
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...
(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.
Depends on: 1315668
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.
Attachment #8808480 - Flags: review?(stransky) → review+
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.
Attachment #8808481 - Flags: review?(stransky) → review+
Blocks: 1303964
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 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 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.
Attachment #8808481 - Flags: review+ → review?(stransky)
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!
Attachment #8808481 - Flags: review?(stransky) → review+
Pushed by ktomlinson@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/fe6045ca3c46
https://hg.mozilla.org/mozilla-central/rev/7a0ad5f1a7f6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1324886
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: