1022127 - Context menu separators are black in gtk3 builds

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Screenshot (left is gtk3, right is gtk2)

See attached screenshot.

Comment 1

[Karl Tomlinson (:karlt)]

GTK3 and GTK2 use different theme engines and perhaps even different themes.

GTK3's default theme uses black menu separators, but that has a grey background (on my system at least), so I assume that is not the theme in the screenshot.

It's hard to tell whether this is a bug, without comparing with a context menu in a native GTK3 app.  e.g. evince.

Comment 2

[Mike Hommey [:glandium]]

(In reply to Karl Tomlinson (needinfo?:karlt) from comment #1)
> GTK3 and GTK2 use different theme engines and perhaps even different themes.
> 
> GTK3's default theme uses black menu separators, but that has a grey
> background (on my system at least), so I assume that is not the theme in the
> screenshot.

The theme in the screenshot is Adwaita (supposedly the default theme), with no customization, on Debian.

> It's hard to tell whether this is a bug, without comparing with a context
> menu in a native GTK3 app.  e.g. evince.

Both roxterm and evince, both using Gtk+3, have a grey separator here.

Comment 3

[Andrew Comminos [:acomminos]]

Attachment: Draw menu separators using the GTK_STYLE_CLASS_SEPARATOR style class on GTK3

Here's a patch for this bug that adds the GTK_STYLE_CLASS_SEPARATOR style class to the GtkStyleContext used to draw the menu separators on GTK3, fixing the issue. This uses the same technique as found in the GTK project's gtk_menu_item_draw function.

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 8459378 [details] [diff] [review]
Draw menu separators using the GTK_STYLE_CLASS_SEPARATOR style class on GTK3

Thank you.

(Perhaps for consistency with gtk_menu_item_draw() and  gtk_menu_item_real_get_height(), GTK_STYLE_CLASS_SEPARATOR should be present during gtk_widget_style_get() [and still not during gtk_style_context_get_padding()], but I don't know whether that is necessary. moz_gtk_get_menu_separator_height() would need to be adjusted consistently.)

Comment 5

[Andrew Comminos [:acomminos]]

Attachment: Updated for consistency with gtk_menu_item_draw.

You're right, it's safer to replicate GTK's implementation in case the added style class overrides those values. Attached is an updated patch with modifications to moz_gtk_get_menu_separator_height as well.

Comment 6

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0243d7236450

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0243d7236450