406676 - GTK Menu Separator is not painted natively

Status: RESOLVED FIXED

Description

[Ian Spence]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007120304 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007120304 Minefield/3.0b2pre

This is a followup to my toolbar separator patch. Currently menu separators are painted as an empty box with a top and bottom border. While this works in general, there are many themes that paint different types of separators, and I've even found ones that painted just a space. So Firefox should use native menu separator painting.

Reproducible: Always

Steps to Reproduce:
1. Open a menu that has a menu separator in it
Actual Results:  
A horizontal line line taking up most of the width of the menu appears where a separator is.

Expected Results:  
Mozilla should paint a native menu separator, taking the width specified by gtk, and having the padding specified by gtk.

Comment 1

[Ian Spence]

Created attachment 291335 [details] [diff] [review]
Paints the menu separator natively

This paints the menu separator natively.

The css change is due to the fact that some gtk themes paint the separator to the full width of the menu. Having any left or right margin prevents this.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+	  guint    horizontal_padding;
+	  gint     paint_height;

Get rid of tabs

+        *size = gMenuSeparatorWidget->style->ythickness * 2;

Why * 2? We're only painting ythickness high.

Comment 3

[Ian Spence]

I was attempting to replicate the set_size_request for the separator

  else /* separator item */
    {
      gboolean wide_separators;
      gint     separator_height;

      gtk_widget_style_get (widget,
                            "wide-separators",  &wide_separators,
                            "separator-height", &separator_height,
                            NULL);

      if (wide_separators)
        requisition->height += separator_height + widget->style->ythickness;
      else
        requisition->height += widget->style->ythickness * 2;
    }

Comment 4

[Ian Spence]

Created attachment 291785 [details] [diff] [review]
Removes the tabs

In case you accept the reasoning behind the doubling of that value, here is a patch with the tabbing fixed.

Comment 5

[Reed Loden [:reed] (use needinfo?)]

Comment on attachment 291785 [details] [diff] [review]
Removes the tabs

More Linux nativity!

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 291785 [details] [diff] [review]
Removes the tabs

a=drivers for when trunk opens after Fx3 Beta 2 freeze

Comment 7

[Reed Loden [:reed] (use needinfo?)]

Checking in toolkit/themes/gnomestripe/global/menu.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/global/menu.css,v  <--  menu.css
new revision: 1.17; previous revision: 1.16
done
Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.48; previous revision: 1.47
done
Checking in widget/src/gtk2/gtkdrawing.h;
/cvsroot/mozilla/widget/src/gtk2/gtkdrawing.h,v  <--  gtkdrawing.h
new revision: 1.41; previous revision: 1.40
done
Checking in widget/src/gtk2/nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.120; previous revision: 1.119
done

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

In de.comm.software.mozilla.nightly-builds, Christoph 'Mehdorn' Weber reported on Nov 16 2007 that he sees messages like the following in his terminal:

Gtk-WARNING **: gtkwidget.c:7322: widget class `GtkToolbar' has no property named `wide-separators'

He seems to be using GTK 2.4 but as far as I can see, the mentioned property was added no earlier than with GTK 2.10 (cf. <http://developer.gimp.org/api/2.0/gtk/ix07.html>). Looking at the patch that was checked in for this bug, I'm pretty sure that this is the culprit. Please confirm. Then, if you want, we can take the issue to a new bug.

Comment 9

[Ian Spence]

From all the docs I've seen, GTK 2.10 is the minimum requirement for Firefox 3

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #9)
> From all the docs I've seen, GTK 2.10 is the minimum requirement for Firefox 3

/Proposed/ minimum requirement, as far as I can see from <http://www.mozillazine.org/talkback.html?article=21748> and <http://wiki.mozilla.org/Linux/Runtime_Requirements>. Maybe you know more than I do here. If it has been finally decided, OK. But if it hasn't, what I brought to your attention might be a bug.