Bug 659993: Part 2: An alternate fix for bug 406129 that avoids the use of hard-coded padding values. r=roc+ventnor

toolkit/themes/gnomestripe/global/menu.css

@@ -54,9 +54,6 @@ menuitem,
   font: menu;
   list-style-image: none;
   -moz-image-region: auto;
-  /* 3px is the default padding value. See the big comment under moz_gtk_check_menu_item_paint()
-     in widget/src/gtk2/gtk2drawing.c and bug 406129 for why we're hardcoding this. */
-  padding: 0px 3px;
 }
 
 menuitem[default="true"] {

widget/src/gtk2/gtk2drawing.c

@@ -858,6 +858,30 @@ moz_gtk_widget_get_focus(GtkWidget* widget, gboolean* interior_focus,
     return MOZ_GTK_SUCCESS;
 }
 
+gint
+moz_gtk_menuitem_get_horizontal_padding(gint* horizontal_padding)
+{
+    ensure_menu_item_widget();
+
+    gtk_widget_style_get (gMenuItemWidget,
+                          "horizontal-padding", horizontal_padding,
+                          NULL);
+
+    return MOZ_GTK_SUCCESS;
+}
+
+gint
+moz_gtk_checkmenuitem_get_horizontal_padding(gint* horizontal_padding)
+{
+    ensure_check_menu_item_widget();
+
+    gtk_widget_style_get (gCheckMenuItemWidget,
+                          "horizontal-padding", horizontal_padding,
+                          NULL);
+
+    return MOZ_GTK_SUCCESS;
+}
+
 gint
 moz_gtk_button_get_default_overflow(gint* border_top, gint* border_left,
                                     gint* border_bottom, gint* border_right)
@@ -2688,7 +2712,7 @@ moz_gtk_check_menu_item_paint(GdkDrawable* drawable, GdkRectangle* rect,
     GtkStyle* style;
     GtkShadowType shadow_type = (checked)?GTK_SHADOW_IN:GTK_SHADOW_OUT;
     gint offset;
-    gint indicator_size;
+    gint indicator_size, horizontal_padding;
     gint x, y;
 
     moz_gtk_menu_item_paint(drawable, rect, cliprect, state, FALSE, direction);
@@ -2698,6 +2722,7 @@ moz_gtk_check_menu_item_paint(GdkDrawable* drawable, GdkRectangle* rect,
 
     gtk_widget_style_get (gCheckMenuItemWidget,
                           "indicator-size", &indicator_size,
+                          "horizontal-padding", &horizontal_padding,
                           NULL);
 
     if (checked || GTK_CHECK_MENU_ITEM(gCheckMenuItemWidget)->always_show_toggle) {
@@ -2706,11 +2731,8 @@ moz_gtk_check_menu_item_paint(GdkDrawable* drawable, GdkRectangle* rect,
       offset = GTK_CONTAINER(gCheckMenuItemWidget)->border_width +
              gCheckMenuItemWidget->style->xthickness + 2;
 
-      /* while normally this "3" would be the horizontal-padding style value, passing it to Gecko
-         as the value of menuitem padding causes problems with dropdowns (bug 406129), so in the menu.css
-         file this is hardcoded as 3px. Yes it sucks, but we don't really have a choice. */
       x = (direction == GTK_TEXT_DIR_RTL) ?
-            rect->width - indicator_size - offset - 3: rect->x + offset + 3;
+            rect->width - indicator_size - offset - horizontal_padding: rect->x + offset + horizontal_padding;
       y = rect->y + (rect->height - indicator_size) / 2;
 
       TSOffsetStyleGCs(style, x, y);

widget/src/gtk2/gtkdrawing.h

@@ -330,6 +330,17 @@ gint
 moz_gtk_widget_get_focus(GtkWidget* widget, gboolean* interior_focus,
                          gint* focus_width, gint* focus_pad);
 
+/** Get the horizontal padding for the menuitem widget or checkmenuitem widget.
+ * horizontal_padding: [OUT] The left and right padding of the menuitem or checkmenuitem
+ *
+ * returns:    MOZ_GTK_SUCCESS if there was no error, an error code otherwise
+ */
+gint
+moz_gtk_menuitem_get_horizontal_padding(gint* horizontal_padding);
+
+gint
+moz_gtk_checkmenuitem_get_horizontal_padding(gint* horizontal_padding);
+
 /**
  * Some GTK themes draw their indication for the default button outside
  * the button (e.g. the glow in New Wave). This gets the extra space necessary.

widget/src/gtk2/nsNativeThemeGTK.cpp

@@ -968,6 +968,23 @@ nsNativeThemeGTK::GetWidgetPadding(nsDeviceContext* aContext,
     case NS_THEME_RADIO:
       aResult->SizeTo(0, 0, 0, 0);
       return PR_TRUE;
+    case NS_THEME_MENUITEM:
+    case NS_THEME_CHECKMENUITEM:
+    case NS_THEME_RADIOMENUITEM:
+      {
+        // Menubar and menulist have their padding specified in CSS.
+        if (!IsRegularMenuItem(aFrame))
+          return PR_FALSE;
+
+        gint horizontal_padding;
+
+        if (aWidgetType == NS_THEME_MENUITEM)
+          moz_gtk_menuitem_get_horizontal_padding(&horizontal_padding);
+        else
+          moz_gtk_checkmenuitem_get_horizontal_padding(&horizontal_padding);
+        aResult->SizeTo(horizontal_padding, 0, horizontal_padding, 0);
+        return PR_TRUE;
+      }
   }
 
   return PR_FALSE;

widget/src/xpwidgets/nsNativeTheme.cpp

@@ -52,6 +52,7 @@
 #include "nsIComponentManager.h"
 #include "nsPIDOMWindow.h"
 #include "nsProgressFrame.h"
+#include "nsIMenuFrame.h"
 
 nsNativeTheme::nsNativeTheme()
 : mAnimatedContentTimeout(PR_UINT32_MAX)
@@ -509,6 +510,14 @@ nsNativeTheme::IsSubmenu(nsIFrame* aFrame, PRBool* aLeftOfParent)
   return PR_FALSE;
 }
 
+PRBool
+nsNativeTheme::IsRegularMenuItem(nsIFrame *aFrame)
+{
+  nsIMenuFrame *menuFrame = do_QueryFrame(aFrame);
+  return !(menuFrame && (menuFrame->IsOnMenuBar() ||
+                         menuFrame->GetParentMenuListType() != eNotMenuList));
+}
+
 PRBool
 nsNativeTheme::QueueAnimatedContentForRefresh(nsIContent* aContent,
                                               PRUint32 aMinimumFrameRate)

widget/src/xpwidgets/nsNativeTheme.h

@@ -172,6 +172,9 @@ class nsNativeTheme : public nsITimerCallback
   // menupopup:
   PRBool IsSubmenu(nsIFrame* aFrame, PRBool* aLeftOfParent);
 
+  // True if it's not a menubar item or menulist item
+  PRBool IsRegularMenuItem(nsIFrame *aFrame);
+
   nsIPresShell *GetPresShell(nsIFrame* aFrame);
   PRInt32 CheckIntAttr(nsIFrame* aFrame, nsIAtom* aAtom, PRInt32 defaultValue);
   PRBool CheckBooleanAttr(nsIFrame* aFrame, nsIAtom* aAtom);