Created attachment 55473 [details]
Example of menus: current status and suggested

Created attachment 55474 [details]
An example of Apple menus (OSX 10.3, found on the Web)

Created attachment 55475 [details]
Another example for Apple Mac OSX showing simple menu entries and check menu entries

PS IMHO should are radio entries....

Created attachment 55497 [details]
New example of menus: current status and suggested

Forgot to mention: good theme engines should provide squares and circles to
suggest Radio and Check menu entries

Created attachment 120973 [details] [review]
patch

Not sure if there is a better way to inspect the style properties without creating the dummy instance.  Looks good though.

bug 412134 has an api proposal to avoid some of the ugliness

I think it would be more elegant to implement 
gtkmenuitem.c:gtk_real_menu_item_toggle_size_request
to return indicator_size + toggle_spacing. 

If you do that, you can remove gtk_check_menu_item_toggle_size_request
at the same time. 

Short of adding the gtk_style_get api referred to above, you can
perhaps use _gtk_style_peek_property_value to avoid the dummy widget.

I thought that it may be better to only perform the lookup once per menu (if necessary) rather than per item.  It is probably more elegant to use the baseclass though.  If we do that then we certainly don't want to be creating a dummy instance to get the property from.

I guess what we really would like here is to have indicator-size be a style property for all menu items and not just check menu items.

It isn't clear to me that _gtk_style_peek_property_value will be that useful if we move the functionality to the baseclass since the property we're interested in is in a subclass.

I meant to pass the subclass to _gtk_style_peek_property_value

Some things we noticed while comparing alternative menu systems:

- If we do this, we probably need to have some padding on the right too, to avoid
  unbalanced appearance (this only seems to affect menus without accels)

- The minimal space between label and accel should probably be adjusted too

Created attachment 121093 [details] [review]
new patch

Maybe moving the style property up the widget hierarchy wouldn't be an API break since it is only used through property accessors?  If so, this patch seems to work.  It should also fix the spacing problem on the other (usually right) side of the menu item label.

Some observations:

- _gtk_style_peek_property_value tries to find style properties for parent types,
  so the gtk_widget_style_get (checkmenuitem, "indicator-size",...) will find
  a value set with GtkMenuItem::indicator-size = x in an rc file.

- The rc file parser doesn't complain about SomeGtkClass::some-property = x even
  if SomeGtkClass doesn't have a style property ::some-property. And the property
  value will be stored in the style and available for lookups.

- Most themes only set GtkCheckMenuItem::indicator-size in styles for check menu items. Those themes will be broken, since the effective padding of a menu will depend on whether it contains check menu items or not.

- Your patch doesn't work right for menus containing only image menu items without images, I think. At least I see some examples of those where the indicator size is added when menu-images is FALSE, but not when it is TRUE.

Created attachment 121715 [details] [review]
new patch

Here is a version of your original patch that uses gtk_style_get().
If we decide to go with this, gtk_style_get should of course be exported and move to gtkstyle.[hc].

The patch doesn't have the deficiencies I mentioned above for the other patch, so I tend towards this one.

(In reply to comment #13)
> Created an attachment (id=121715) [edit]
> new patch
> 
> Here is a version of your original patch that uses gtk_style_get().
> If we decide to go with this, gtk_style_get should of course be exported and
> move to gtkstyle.[hc].

I didn't really look this trough in detail, but I suggest to be careful with enhancing the current GtkStyle interface. There are people working heavily on finding out if replacing GtkStyle is a good idea. Granted it won't just disappear, but maybe _gtk_style_get should be an internal, underscore prefixed function, at least for the moment.

You've said as much in the other bug. 

However, I disagree with you assessment that this adds any new functionality that would be a burden to maintain for HotNewStyle. You can see that by looking at 
Jons original patch: You can already get the style properties, at the cost of instantiating widgets all the time.

(In reply to comment #15)
> You've said as much in the other bug. 
> 
> However, I disagree with you assessment that this adds any new functionality
> that would be a burden to maintain for HotNewStyle. You can see that by looking
> at Jons original patch: You can already get the style properties, at the cost of
> instantiating widgets all the time.

My primary point of concern is actually not maintenance. But in the case that we actually end up with a new style interface it would be a shame to have introduced an enhancement only to deprecate it. It would hardly be worth porting code if it's deprecated already in the next version. So from the perspetive of a developer using Gtk it would feel awkward.

That said, it may be of more use than I realize now, so my opinion is not totally carved in stone :)

Bug 322934 – Replace menu's proxy icons with empty space hiding icons

* gtk/gtkmenu.c (gtk_menu_size_request): Use consistent padding
regardless of imagees or checks being in the menu. Also add
padding on the right edge.
Proposal by Luca Ferretti, patch by Jon McCann

*** Bug 561521 has been marked as a duplicate of this bug. ***

Doing this unconditionally makes small multicolumn menus really ugly.  See attachment 137130 [details] in bug 585421.  Please add a way to request the original smart behaviour.

(In reply to comment #19)
> Please add a way to request the original smart behaviour.

This is easy if one really has a GtkMenu.  However, the ugly screenshot shows a GtkComboBox which does not provide any means to access the GtkMenu widget.

But maybe the new behaviour makes little sense for menus created from a GtkComboBox anyway.  Combo boxes with icons are relatively rare and those that have icons tend to have them in each item.  So simply reverting the behaviour for all combo box menus could be acceptable.

> So simply reverting the behaviour
> for all combo box menus could be acceptable.

I've done that yesterday