GNOME Bugzilla – Bug 708905
gtk_menu_bar_new_from_model() should be private or better documented
Last modified: 2018-04-15 00:18:53 UTC
gtk_menu_bar_new_from_model() can create a GtkMenuBar from a GMenu, such as one given by GtkBuilder. I can pack that into a GtkBox in a GtkWindow and it will mostly work. However, there is then no way to tell the window about the menu's accelerators, such as Ctrl-C. With GtkUIManager, we did this with: gtk_widget_add_accel_group (gtk_ui_manager_get_accel_group (uimanager)) . I could use gtk_application_add_accelerator() but that would duplicate the GtkBuilder's "accel" attribute XML syntax. (bloatpad.c uses gtk_application_add_accelerator() for some reason, even though it also specifies the same accelerator in the <menu> XML.) As suggested by Ryan on irc, if I instead specify the GMenu as an application-wide per-window menu with gtk_application_set_menubar(), then the accelerators will work. If that's the only way to use a GMenu to create a window's menubar, then gtk_menu_bar_new_from_model() does not seem useful for anything outside of GTK+ itself. I'm also concerned that I have no way with GMenu to have different menubars in different windows. For instance, my app opens a secondary window for editing a layout on a canvas, and this UI requires several menu items.
(In reply to comment #0) > I could use gtk_application_add_accelerator() but that would duplicate the > GtkBuilder's "accel" attribute XML syntax. > (bloatpad.c uses gtk_application_add_accelerator() for some reason, even though > it also specifies the same accelerator in the <menu> XML.) Strangley, using gtk_application_add_accelerator() doesn't seem to work with a even as a workaround, but I guess I need to create a test case for that.
(In reply to comment #1) > Strangley, using gtk_application_add_accelerator() doesn't seem to work with a > even as a workaround, but I guess I need to create a test case for that. Of course that was wrong. I was using a GtkWindow (from a .glade file) as a GtkApplicationWindow.
By the way, there is now also gtk_application_set_accels_for_action() though there's no hint about how that relates to gtk_application_add_accelerator().
gtk_application_set_accels_for_action is doing the same as gtk_application_add_accelerator, with the difference that it lets you specify multiple accels for the same detailed action. As documented, the accel attributes in the xml get only interpreted by gtk_application_set_app_menu and gtk_application_set_menubar. The accel attribute will be replaced by a more flexible mechanism in the future,
gtk_application_add_accelerator() uses accels_set_accels_for_action() after getting the detailed action name, suggesting that it replaces any previously-added accelerator. If that's intended, I guess it should be documented. Or maybe we could just deprecate gtk_application_add_accelerator() because it confuses the API.
Deprecation makes sense to me. This function is very confusingly named, and we can't really change it without breaking backcompat (which is why I left it alone when I added the new API).
btw: the more-flexible mechanism from the future is already here. These days, strictly speaking, accel='' is simply a directive to menu-drawing code to hardcode an accel label onto a menu item. This is no longer required or indeed desired if you want the accel label to be able to change (due to user preference). If you leave off the accel='' attribute then the accel will come from the GtkApplication, as long as the application is visible to the menu, in the same sense that the application's actions are visible (ie: put the menu into a widget tree associated with the app). accel= is read off of menus set with _set_app_menu() or _set_menubar() but this is really just a "helper" feature. You should probably not set this attribute at all anymore. The _set_accels() API is really the "official"/"best" way. The only future improvement here is basically just the idea that we will soon be able to have an XML file with these accels instead of calling the function a bunch of times. We will probably also add some mechanism for user overrides of accels at this point as well (either via dconf or keyfiles).
I've deprecated gtk_application_add_accelerator now
Thanks, but you might want to let gtk_application_set_accels_for_action() accept NULL so it can be a replacement for the now-deprecated gtk_application_remove_accelerator(). See bug #726623 .
You can pass the empty array in order to set no accels. This might sound difficult, but when you think about it, this API is mainly directed at a graphical "accel editor" sort of UI, and for that use it makes a good deal of sense.
Would you accept a patch for NULL to do the same thing? Not accepting NULL seems unnecessarily awkward.
Au contraire - confusing NULL with a valid, empty object whatever type you have at hand is what is awkward and should be avoided in GTK+ apis. empty string != NULL empty array != NULL keep this sort of 'convenient confusion' in scripting languages, please.
As usual, I disagree about this, of course. But do you at least want a check to warn instead of crashing, as mentioned in bug #726623 .
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new