Closed Bug 406676 Opened 17 years ago Closed 17 years ago

GTK Menu Separator is not painted natively

Categories

(Core :: Widget: Gtk, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: ispence, Assigned: ispence)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attachment #291335 - Flags: superreview?(roc)
Attachment #291335 - Flags: review?(roc)
Version: unspecified → Trunk
+	  guint    horizontal_padding;
+	  gint     paint_height;

Get rid of tabs

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

Why * 2? We're only painting ythickness high.
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;
    }
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee: nobody → ispence
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Attached patch Removes the tabsSplinter Review
In case you accept the reasoning behind the doubling of that value, here is a patch with the tabbing fixed.
Attachment #291335 - Attachment is obsolete: true
Attachment #291785 - Flags: superreview?(roc)
Attachment #291785 - Flags: review?(roc)
Attachment #291335 - Flags: superreview?(roc)
Attachment #291335 - Flags: review?(roc)
Attachment #291785 - Flags: superreview?(roc)
Attachment #291785 - Flags: superreview+
Attachment #291785 - Flags: review?(roc)
Attachment #291785 - Flags: review+
Comment on attachment 291785 [details] [diff] [review]
Removes the tabs

More Linux nativity!
Attachment #291785 - Flags: approval1.9?
Comment on attachment 291785 [details] [diff] [review]
Removes the tabs

a=drivers for when trunk opens after Fx3 Beta 2 freeze
Attachment #291785 - Flags: approvalM10-
Attachment #291785 - Flags: approval1.9?
Attachment #291785 - Flags: approval1.9+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Component: GFX: Gtk → Widget: Gtk
Keywords: checkin-needed
QA Contact: gtk → gtk
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
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.
From all the docs I've seen, GTK 2.10 is the minimum requirement for Firefox 3
(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.
Depends on: 1294136
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: