GNOME Bugzilla – Bug 789268
Keyboard grab by popup window causes a session-modal shortcut-inhibition dialog
Last modified: 2018-05-03 08:52:37 UTC
Clicking on the smiley picker on GNOME 3.26 results in a session-modal prompt that "Evolution wants to inhibit shortcuts" (from Mutter apparently). It's not clear why this is, since it's not necessary for smiley selection whether by mouse or keyboard.
That's gtk+/gdk issuing a "shortcut inhibit" request under Wayland when the apps tries to use the gdk_device_grab() api, see: https://git.gnome.org/browse/gtk+/commit/?h=gtk-3-22&id=1c23bce3 Apart from questioning the need to use a keyboard grab in this case for evo, this should be moved to gtk+ as I see three possibilities: 1. Adapt evo to use the new gdk_seat_grab() api that lets the caller grab both the keyboard and pointer at once. => gdk wayland backend will not issue a "shortcut inhibit" request if both keyboard and pointer are being grabbed, so that would avoid the issue. 2. Change gdk wayland backend to not issue the "shortcut inhibit" request from the old gdk_device_grab() and gdk_keyboard_grab() apis. => But that means that other applications such as virt-manager which really need the shortcut inhibit request would need to be adapted/ported to the new api to benefit from the feature under Wayland. 3. Add a new api altogether in gdk to issue the shortcut inhibitor request (which would be a no-op on all other backends) and unwire it entirely from the existing grab apis. => but again that means porting existing apps that need it to that new api. => That also means adding a new api to gtk 3.22 which is not suppose to see new api (yet we just added the emojichooser api to gtk3 so this /might/ be possible) So my advise here would be to move this bug to gtk+ to discuss the points listed above (assuming the keyboard grab is relevant in evo).
Thanks for the detailed reply :) TBH my bug is more about "why does Evo need to do this at all in this case" - denying the request doesn't seem to break anything when picking a smiley via keyboard or mouse, which suggests the gdk_device_grab() call is unnecessary for the smiley picker.
Thanks for a bug report. There are more places where gdk_device_grab() is used: $ git grep -c gdk_device_grab src/calendar/gui/e-calendar-view.c:1 src/calendar/gui/e-day-view-time-item.c:1 src/calendar/gui/e-day-view.c:4 src/calendar/gui/e-week-view.c:1 src/e-util/e-cell-combo.c:2 src/e-util/e-color-combo.c:2 src/e-util/e-dateedit.c:2 src/e-util/e-emoticon-tool-button.c:2 src/e-util/e-name-selector-list.c:2 src/libgnomecanvas/gnome-canvas.c:3 As we had a chat with Olivier on IRC, evo should either not grab the keyboard or it may use gdk_seat_grab(..., GDK_SEAT_CAPABILITY_POINTER | GDK_SEAT_CAPABILITY_KEYBOARD) instead, which instructs GDK not to issue the "shortcut inhibitor" request under Wayland.
I tried to change this, but as there are many places and the widgets are rather old, the attempt to remove keyboard grabs led to breakage (when I can alt+tab elsewhere, then the date picker in File->New->Appointment->Start time->Click-first-dropdown-button, then even a window of the other application can be covered by this date picker, which is wrong). I found some other unrelated issues when looking around the places, which I'll fix separately.
I think this is yet another example of why it might not have been the best idea to reuse "grab keyboard only" => "inhibit shortcuts" instead of biting the bullet and introducing an API that does what we need. The question has been raised weather to remove the "allow shortcut inhibitation" dialog and just grant-by-default. I don't think that is a good idea, as it'd effectively mean all these old dialogs that take a keyboard grab would effectively take that exact grab even on Wayland, while the fact is that we have tried our best to *not* do exactly that. There are relatively few places where an actual keyboard shortcut inhibitation is actually needed, and if we want to support that without all these side effects, I still think we should reconsider API for this even in gtk 3.22 so things like virt-manager et al can still work properly.
(In reply to Jonas Ådahl from comment #5) > ... it'd effectively mean all these old dialogs that take a keyboard > grab ... Just a little clarification, in the evolution case, it's not a dialog which grabs focus, it's a widget which tries to simulate combo box (evolution doesn't use GtkPopover, neither it would fit for this purpose). User clicks a button which looks like a button of a combo, with the arrow down icon, and it shows a popup window with some other widgets, like the date picker. Once the popup is closed the grab on both keyboard and mouse is released. The current situation is that when I Deny the shortcut inhibit, then I can Alt+Tab away from the popup to a different application, which is not how X11 or GtkCombobox works. One side effect of this Alt+Tab is that the date-picker popup of the Evolution is left on top of everything, thus even the other window hides content of the dialog where the button to show the popup had been clicked, it doesn't get above the popup itself, thus the popup window hides content of the other application window. I can make a screenshot, if it would help to understand what I try to describe here.
(In reply to Milan Crha from comment #6) > (In reply to Jonas Ådahl from comment #5) > > ... it'd effectively mean all these old dialogs that take a keyboard > > grab ... > > Just a little clarification, in the evolution case, it's not a dialog which > grabs focus, it's a widget which tries to simulate combo box (evolution > doesn't use GtkPopover, neither it would fit for this purpose). User clicks > a button which looks like a button of a combo, with the arrow down icon, and > it shows a popup window with some other widgets, like the date picker. Once > the popup is closed the grab on both keyboard and mouse is released. Ah, I see. That is yet another use case which shouldn't trigger shortcut inhibitation. > > The current situation is that when I Deny the shortcut inhibit, then I can > Alt+Tab away from the popup to a different application, which is not how X11 > or GtkCombobox works. One side effect of this Alt+Tab is that the > date-picker popup of the Evolution is left on top of everything, thus even > the other window hides content of the dialog where the button to show the > popup had been clicked, it doesn't get above the popup itself, thus the > popup window hides content of the other application window. I can make a > screenshot, if it would help to understand what I try to describe here. The ability to Alt-Tab away is expected, and I think that is the behaviour that makes sense for more or less all situations except virtual machine viewers etc. With that said, it seems the window dismiss code relies on the X11 style grab being handled, which is not something we want to allow under Wayland. The date-picker popup, what is it made out of? It sounds like it is not turned into a popup in the GDK backend, but it probably should have.
AFAICT we aren't doing any window checks when triggering shortcut inhibition on the GTK+ wayland backend, presumably we can avoid that entirely for popups? I'm attaching an untested patch in that direction. Olivier, please confirm or deny this should cater for the existing usecases :).
Created attachment 362325 [details] [review] gdk/wayland: Restrict shortcut inhibition to keyboard grabs on toplevels It is unlikely that popup windows will contain anything that requires this (popup menus being more interested in redirecting keyboard focus to themselves). OTOH popup implementations that just grab the keyboard are commonplace enough, it makes sense not to trigger inhibition for these.
(In reply to Carlos Garnacho from comment #8) > AFAICT we aren't doing any window checks when triggering shortcut inhibition > on the GTK+ wayland backend, presumably we can avoid that entirely for > popups? I'm attaching an untested patch in that direction. > > Olivier, please confirm or deny this should cater for the existing usecases > :). Most important use case is the keyboard shortcut dialog in gnome-control-center that needs the shortcut inhibition mechanism, restricting to toplevel may affect that... (untested)
Review of attachment 362325 [details] [review]: I think a similar test should be done when invoking gdk_wayland_window_restore_shortcuts() on ungrab()
(In reply to Jonas Ådahl from comment #7) > The date-picker popup, what is it made out of? It sounds like it is not > turned into a popup in the GDK backend, but it probably should have. It's just one example (see comment #3, it's the e-dateedit.c in the list). It is created as a GTK_WINDOW_POPUP GtkWindow: https://git.gnome.org/browse/evolution/tree/src/e-util/e-dateedit.c?h=gnome-3-24#n652 Then it is shown, and the grabs are done, here: https://git.gnome.org/browse/evolution/tree/src/e-util/e-dateedit.c?h=gnome-3-24#n1444 Finally, it's hidden and grabs are removed here: https://git.gnome.org/browse/evolution/tree/src/e-util/e-dateedit.c?h=gnome-3-24#n1674 Most of that code is very old and if I'm not mistaken, then it had been "copied" from (or at least made very similar to) GtkCombox itself, in time when it had been done (as it wants to simulate the combo box popup behaviour).
(In reply to Olivier Fourdan from comment #11) > Review of attachment 362325 [details] [review] [review]: > > I think a similar test should be done when invoking > gdk_wayland_window_restore_shortcuts() on ungrab() Well, actually gdk_wayland_window_restore_shortcuts() will not complain and not issue a shortcut_inhibitor_destroy() if there is not keyboard inhibited, so it is safe to call unconditionally. Also I checked with gnome-control-center's keyboard shortcut cinfigration panel and it's working fine with your patch so I think this would work as a viable workaround for the issue.
I am tempted to say let's use this as a workaround (that will fix evo and possibly others w/out breaking other apps that need shortcuts inhibition) while the decision to keep or remove the dialog in mutter/gnome-shell is left pending?
(In reply to Milan Crha from comment #12) > (In reply to Jonas Ådahl from comment #7) > > The date-picker popup, what is it made out of? It sounds like it is not > > turned into a popup in the GDK backend, but it probably should have. > > It's just one example (see comment #3, it's the e-dateedit.c in the list). > It is created as a GTK_WINDOW_POPUP GtkWindow: > https://git.gnome.org/browse/evolution/tree/src/e-util/e-dateedit.c?h=gnome- > 3-24#n652 > Then it is shown, and the grabs are done, here: > https://git.gnome.org/browse/evolution/tree/src/e-util/e-dateedit.c?h=gnome- > 3-24#n1444 Porting to gdk_seat_grab() is still highly recommended :), it makes both showing the popup window and grabbing the input devices "atomic" from the caller perspective. The GDK wayland backend can barely make sense of the three calls done separately. Seeing that you actually grab both pointer and keyboard (I oversaw that the GdkDevice::grab vfunc on the keyboard would still trigger shortcut inhibition despite the pointer grab), a gdk_seat_grab() call on GDK_SEAT_CAPABILITY_ALL would have resulted on correct behavior. (In reply to Olivier Fourdan from comment #14) > I am tempted to say let's use this as a workaround (that will fix evo and > possibly others w/out breaking other apps that need shortcuts inhibition) > while the decision to keep or remove the dialog in mutter/gnome-shell is > left pending? I'll be more romantic and call it "heuristic" :p, IMHO the dialog is good to have, but it's unlikely that a popup window actually demands the functionality. Seeing that it may also happen with the traditional 2 gdk_device_grab() calls, I think something must be done in GTK+, indeed. Will move the bug there.
Carlos, do you want my R-b for the gtk+ patch (attachment 362325 [details] [review]) or do you prefer to double check with Matthias first?
Review of attachment 362325 [details] [review]: LGTM. That's simple enough and avoids the issue with popup windows.
*** Bug 790930 has been marked as a duplicate of this bug. ***
*** Bug 791008 has been marked as a duplicate of this bug. ***
Oops, I let this slip by... Now on gtk-3-22. Not pushing to master since individual device grabs shall/should probably go. Attachment 362325 [details] pushed as e7e047f - gdk/wayland: Restrict shortcut inhibition to keyboard grabs on toplevels
(In reply to Carlos Garnacho from comment #20) > Oops, I let this slip by... Now on gtk-3-22. Not pushing to master > since individual device grabs shall/should probably go. Then we need to come up with a specific API to inhibit shortcuts in master (which is something we discussed before with Jonas iirc). Filed bug 791057 for this.
*** Bug 791143 has been marked as a duplicate of this bug. ***
Still happening in Evolution 3.28.1 (Fedora 28). I've also commented in the following bug report, which seems to be closely related: https://bugzilla.gnome.org/show_bug.cgi?id=792599