GTK Menus have the option for vertical padding, which some popular themes do take advantage of. Currently padding for a menu is set to 0 pixels all around. The top and bottom padding should take from the GTK style. Attached is an image showing the problem. On the top is a native GTK application menu. In the middle is how Firefox currently renders the menu. On the bottom is how it renders with the pending patch.

Actually a pretty straight forward patch. I look up the vertical padding, and set it in nsNativeThemeGTK::GetWidgetPadding.

Comment on attachment 291783 [details] [diff] [review] Patch to give menus native vertical padding *wonders if he's allowed to request approval1.9*

Comment on attachment 291783 [details] [diff] [review] Patch to give menus native vertical padding a=drivers for after the Firefox 3 Beta 2 freeze

Comment on attachment 291783 [details] [diff] [review] Patch to give menus native vertical padding >Index: widget/src/gtk2/nsNativeThemeGTK.cpp >+ if (aWidgetType == NS_THEME_MENUPOPUP) { >+ gint vertical_padding; >+ moz_gtk_get_menu_popup_vertical_padding(&vertical_padding); >+ >+ aResult->SizeTo(0, vertical_padding, 0, vertical_padding); >+ return PR_TRUE; >+ } The indention is wrong. Should be two spaces in this file.

The same the the last patch, but with reduced tabs. Do I need to re-request approval?

(In reply to comment #5) > Do I need to re-request approval? Nope. Once the tree reopens, I'll commit the change.

Checking in widget/src/gtk2/gtk2drawing.c; /cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v <-- gtk2drawing.c new revision: 1.49; previous revision: 1.48 done Checking in widget/src/gtk2/gtkdrawing.h; /cvsroot/mozilla/widget/src/gtk2/gtkdrawing.h,v <-- gtkdrawing.h new revision: 1.42; previous revision: 1.41 done Checking in widget/src/gtk2/nsNativeThemeGTK.cpp; /cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v <-- nsNativeThemeGTK.cpp new revision: 1.121; previous revision: 1.120 done

This should probably be backed out until I can figure out how to determine how to be sure it is a dropdown from the menubar/toolbar (or a submenu of one). This padding, while it is the correct thing to do, it being exposed as a problem with the fix of bugs like bug 398242

Backed out.

Since bug 398242 is obsolete, can someone take a look at this again? This is very annoying for me.