GNOME Bugzilla – Bug 516425
Optionally display accelerators in popups
Last modified: 2008-12-19 11:10:01 UTC
GtkUIManager-created <popup> menus don't show accelerators. While this policy is completely reasonable on the desktop, it is a *policy* that is inappropriate on certain platforms (e.g. embedded systems, in this case maemo) where the platform might choose to have a popup menu as toplevel menu (instead of a menu bar). Attached patch allows to have accels in individual popup menus, by either saying <popup accelerators="true"> in the xml or using GTK_UI_MANAGER_POPUP_WITH_ACCEL in gtk_ui_manager_add_ui().
Created attachment 105220 [details] [review] Patch implementing the above
Mitch, what did we discuss about this patch ? I think we wanted to make ui manager swallow unknown attributes.
Yes we wanted to do that. But in the meantime I wonder if that shouldn't be simply a boolean property on GtkAction, just as there are many other obscure special-case properties. As in: g_object_set (action, "popup-accelerators", TRUE, NULL); Which would require people to keep around a special action for the popup menu, which is probably not asked too much. I'm really not sure what the "right" way is here.
I am working on the GIMP help browser currently and there it would be really nice to have this feature. The browser window is supposed to be very compact and I don't want to add a menubar as most functionality is accessible from the toolbar already. For the sake of discoverability it would be nice if the right-click popup menu showed the accelerators though. I favor the approach of specifying this in the UI manager XML. In my opinion this is an option of the view. Making it a property of the model (the action), would be wrong, IMO.
Going through the three options you suggested: > GTK_UI_MANAGER_POPUP_WITH_ACCEL That seems a bit awkward. All of the present types map to widget types. This one would only be a special cased variant of GTK_UI_MANAGER_POPUP. > <popup accelerators="true"> in the xml Looks fine in the markup. If you know at creating time of the popup whether you need accelerators or not, this could work. > g_object_set (action, "popup-accelerators", TRUE, NULL); This is, imho, too specialized, compared to what GtkAction currently has. Even compared to the less generic properties. On the other hand, if you need to show or hide accelerators at runtime, it's the only viable way I can see.
re comment #2, we skip unknown attributes now, which is the right thing to do regardless of whether this patch is right or not: 2008-06-13 Michael Natterer <mitch@imendio.com> * gtk/gtkuimanager.c (start_element_handler): silently skip unknown attributes instead of bailing out with an error in order to be compatible with possible future attribute names. This is related to the discussion in bug #516425 but actually needed for any kind of XML format extension.
Downstream bug: https://bugs.maemo.org/show_bug.cgi?id=2281
Created attachment 119222 [details] [review] Updated patch Same patch, but applies without conflicts on top of trunk.
Ok, lets do this, I guess.
Fixed in SVN: 2008-10-09 Michael Natterer <mitch@imendio.com> Bug 516425 – Optionally display accelerators in popups * gtk/gtkuimanager.h (enum GtkUIManagerItemType): add value GTK_UI_MANAGER_POPUP_WITH_ACCELS which works like _POPUP but shows the actions' accelerators. * gtk/gtkuimanager.c: honor the new enum value for programmatically created UIs, and support <popup accelerators="true"> in the XML for the same purpose. 2008-10-09 Michael Natterer <mitch@imendio.com> Bug 516425 – Optionally display accelerators in popups * gtk/tmpl/gtkuimanager.sgml: document the enhanced XML syntax and the new enum value for popups with accelerators.