GNOME Bugzilla – Bug 694160
MenuButton popup menu captures keys stopping activatable via accelerator
Last modified: 2018-04-15 00:00:40 UTC
Created attachment 236752 [details] [review] experiment with looking up action and key press The menu button implements GtkActionable and GtkActivatable if you associate an accelerator key with this to toggle the menu button you can activate but not de-activate the button via the key. As seen in https://bugzilla.gnome.org/show_bug.cgi?id=686755 This is because the key events are swallowed by the popup menu once it has been activated. Attached is an idea on how this might be solved by listening to the popup menu key presses and looking up to see if there is a corresponding action/key combination and then activating it. I'm not sure who should have priority on the keys, if you set an accelerator on the popup menu which is the same as one associated with the action. Also the implication of the parameters to the action. It may be possible to redirect the events that aren't eaten by the popup menu to somewhere which will recognise the accelerator key.
Created attachment 236753 [details] Test case
Review of attachment 236752 [details] [review]: ::: gtk/gtkmenubutton.c @@ -442,1 +448,1 @@ } This is something that should be added regardless, you can currently call g_action_activate (action, NULL); from anywhere and the button will be toggled off but the menu will still visible.
(In reply to comment #2) > This is something that should be added regardless, you can currently call > g_action_activate (action, NULL); from anywhere and the button will be toggled > off but the menu will still visible. Makes sense - can you please split this into a separate patch?
Review of attachment 236752 [details] [review]: ::: gtk/gtkmenubutton.c @@ +655,3 @@ + if (action_name) + if ((window = gtk_widget_get_toplevel (GTK_WIDGET (menu_button)))) + const gchar *action_name; I don't think the action lookup is needed...you should use code similar to what gtk_application_add_accelerator() does - something like gchar *accel_path = _gtk_accel_path_for_action (action_name, NULL); if (gtk_accel_map_lookup_entry (action_path, &key)) { // etc... } I see the problem with the parameter, but I don't think we can do much about it. I'd welcome the opinion of Ryan on this.
(In reply to comment #3) > (In reply to comment #2) > > > This is something that should be added regardless, you can currently call > > g_action_activate (action, NULL); from anywhere and the button will be toggled > > off but the menu will still visible. > > Makes sense - can you please split this into a separate patch? Ok, filed as bug 694189
Why would you want to use the same accel to popdown the menu instead of just escape?
Why wouldn't you? I think it makes sense to expect the same key to do the inverse action if the button is a toggle.
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