GNOME Bugzilla – Bug 342123
"On top" checkbox only drawn when selected
Last modified: 2006-11-15 16:16:32 UTC
Select Glider theme (for example). Open the window menu for any window. Note that, unlike other unselected checkbox menu items in other applications, no empty checkbox is drawn beside the "on top" menu item. The correct, gtk-themed checkbox is only drawn when "on top" is selected.
Seems like Metacity badly assumes that if the checked attribute of MenuItem is FALSE it assumes it is a regular item and doesn't draw the checkbox. The way I have solved this problem is to add a MetaMenuItemType type attribute to MenuItem which declares what kind of MenuItem to draw. I then changed much code so that instead of tests like if (menuitems[i].stock_id) you compare it against the type: if (menuitem.type == MENU_ITEM_IMAGE). That makes the code easier to understand IMHO. The patch also includes a fix for #343108 since that was so easy and the bugs are related and some refactorings (like for loops instead of while loops). Attaching the new menu.c too because the patch is hard to read.
Created attachment 66974 [details] [review] Introduce MetaMenuItemType enum, change declaration of menu_item_new, fix 342123 + 343108
Created attachment 66975 [details] menu.c with patch attached
Created attachment 66976 [details] menu.c with patch applied
I tried to look over it quickly. General idea seems good. Just a couple smaller issues: - You forgot to use -p when making your patch. ;-) - I think it'd be a little easier to read if the MetaMenuOp came before the MetaMenuItemType in the MenuItem struct. - As per Calum's comments in bug 343108 comment 4, I don't think using a checkbox for the Always on visible workspace thing is a good idea.
Created attachment 74675 [details] [review] Björn's patch reviewed also prepared to solve bug #343108 soonly (using radio buttons).
Created attachment 74851 [details] [review] patch this patch fixes bug #342123 and bug #343108.
Created attachment 74901 [details] [review] patch (I forgot a detail in previous) fixes this and the #343108
Looks good to me, and works fine. Can you commit this or shall I?
I don't have a CVS account yet... you can commit.
Can I close this and the bug #343108?
I'll open a bug to insert these features in libwnck too.
Committed. Feel free to close these two bugs. Thank you!