GNOME Bugzilla – Bug 159890
GtkMenu: should be possible to disable keyboard grab
Last modified: 2011-02-04 16:18:41 UTC
In order to implement a VKB with a context menu it must be possible to disable the keyboard grab. Otherwise the menu steals the keyboard focus away from the application and at least with GtkEntry case the IM context is reset when the focus returns (ie. the menu is dismissed) screwing up the VKB state.
Created attachment 34286 [details] [review] proposed patch Add gtk_menu_set_grab_keyboard() that sets 'accept-focus' for menu->toplevel which is checked on popup() to determine whether or not to grab the keyboard.
Created attachment 34935 [details] [review] another try As discussed on IRC renamed the API to gtk_menu_set_take_focus() and storing the flag value in GtkMenuPrivate::take_focus rather than relying on the window's 'accept-focus' property.
Discussed this with owen some more, and the outcome is that the patch should be safe and not cause problems. The documentation could do with minor improvements: - Change "Since 2.6" to "Since 2.8 - It would perhaps be good to explicity say this is intended only for onscreen keyboards and other special utilities which need to display menus without taking the keyboard focus away from other apps. - It would also be good to describe the consequences of setting this to FALSE in some more detail: The menu does not grab the keyboard, so * if the focus is in some other app, it keeps the focus and keynav in the menu doesn't work. * if the focus is on some toplevel owned by the onscreen keyboard, keynav in the menu will work. - It may actually be better to recommend creating menus without mnemonics and accelerators in this case, so that they are not misleadingly displayed when they don't work.
Created attachment 39008 [details] [review] Revised patch Attached patch fixes the issues mentioned in Matthias' comment. Is this patch OK to commit?
Looks good to me, basically. Some more comments: - Should take-focus be a property ? - Do we need take-focus only on menus, or also on menubars ? - Relatedly, is take-focus inherited by submenus ?
- Should take-focus be a property ? Yes, I think it should be a property. Will update the patch accordingly. - Do we need take-focus only on menus, or also on menubars ? I don't think it's a problem with menubars, since they are part of normal windows, so grab problems should not occur. Or do I miss something? - Relatedly, is take-focus inherited by submenus ? I guess that would be useful. An open question would be if it's propagated to existing submenus or only to newly created ones? What do you think.
The question for menubar was really about inheritance as well. If we decide that take-focus should be inherited, then it would make sense to allow setting take-focus on the menubar, so that all menus can inherit it from there. I don't know if onscreen keyboards ever have menubars, though.
Ah, right :) Which brings up question 3 again. Should it only propagate upon adding new submenus or should it walk the entire tree once the property is set? The latter behavior would probably be very useful when creating the hierarchy using a GtkUIManager. Picking each submenu would be quite a pain for large hierarchies.
Walking the tree would probably be better, although you could almost get away with only propagating down when the submenu is about to be popped up
Created attachment 39446 [details] [review] Add "take-focus" property Attached patch adds the "take-focus" property.
WRT propagating the property to submenus, it appears that adding some lines of code to _gtk_menu_item_popup_submenu() should be enough (and probably much less dangerous than actually walking the tree, which would have to be done whenever the menu tree gets modified). The only case this approach would not catch is if the application picks up a submenu manually and calls gtk_menu_popup() itself. Can we simply ignore that pathologic case?
I think ignoring that case (or maybe mentioning it in the take-focus documentation) should be fine.
Ok, I added the bits of code in my tree and it works fine. For the menubar issue, wouldn't it make sense to just move the take-focus property and API from GtkMenu to GtkMenuShell? It would work transparently for menus and menubars then without any change to the few lines of code in _gtk_menu_item_popup_submenu().
Created attachment 39504 [details] [review] Move API to GtkMenuShell and propagate the property Well, I decided it makes a whole lot of sense to move the API to GtkMenuShell :-) Attached patch does this, adds propagation to submenus by changing _gtk_menu_item_popup_submenu(), and moves the call to gtk_window_set_accept_focus() to gtk_menu_popup(). After this change it's sufficient to set "take-focus" on the toplevel menubar and it will be properly set for all submenus upon popup.
Created attachment 39515 [details] [review] Next iteration incorporating Matthias' comments on irc
Looks good, please commit.
Fixed in CVS HEAD: 2005-03-31 Michael Natterer <mitch@gimp.org> Allow to pop up menus without grabbing the keyboard. Useful for stuff like virtual keyboards. Fixes bug #159890 * gtk/gtk.symbols * gtk/gtkmenushell.[ch]: added boolean property "take-focus" and public API gtk_menu_shell_set/get_take_focus(). * gtk/gtkmenu.c (gtk_menu_popup) (popup_grab_on_window): don't grab the keyboard if take_focus is FALSE. * gtk/gtkmenuitem.c (_gtk_menu_item_popup_submen): propagate the parent menu_shell's take_focus property to the submenu which is about to be popped up.