GNOME Bugzilla – Bug 771694
GtkSourceView completion popup window not shown, no grabbed seat found
Last modified: 2016-10-20 09:40:17 UTC
Created attachment 335910 [details] WAYLAND_DEBUG=1 gnome-builder On wayland with mutter-3.22, gtk+-3.22 & gtksourview-3.22 no completion popup is shown ever in builder or gedit with the following message: (gnome-builder:792): Gdk-WARNING **: No grabbed seat found, using the default one in order to map popup window 0x375d330. You may find oddities ahead, gdk_seat_grab() should be used to simultaneously grab input and show this popup I'm pretty sure this issue started for me with the move to xdg_shell_v6 spec, so I've attached a wayland debug log, search for "grabbed seat".
The completion window is of type GTK_WINDOW_POPUP. GtkSourceCompletion is like GtkEntryCompletion. So maybe there is the same problem with GtkEntryCompletion. Or if the problem is fixed for GtkEntryCompletion, then GtkSourceCompletion should do the same.
I think this happens on Xorg too. Although, the undefined behavior seems to be a bit different in that I can get the items to autocomplete even do I can't see the window.
(In reply to Marinus Schraal from comment #0) > On wayland with mutter-3.22, gtk+-3.22 & gtksourview-3.22 no completion > popup is shown ever in builder or gedit On Fedora 24 with up-to-date GTK+ etc in jhbuild, the completion window appears correctly. Does the bug occur only with mutter 3.22?
As said in #0 it started with the move to v6 spec, that was 3.21.91 afaik. gtk-demo gtkentrycompletion from git head (3.22.1) works fine on wayland, gsv completion still doesn't.
Moving from GtkSourceView to GTK+, because it is an API break.
Well, the logs show that, actually, gtk+ does setup the xdg_popup just it did in gtk-3.20, but mutter/gnome-shell dismisses it, so I am not convinced this ia regression in gtk+ itself.
Ah no sorry, I take that back... gtk+ withdraw the popup
Right, making some progress, actually, this is a focus issue, with xdg_shell v6 we now get a focus-out event when the popup is mapped in Wayland, to which gtksourceview will respond with an unmap of the popup... - in focus_out_event_cb() from gtksourcecompletioninfo.c:211 - in hide_completion_cb() from gtksourceview/gtksourcecompletion.c:1803 And the funny thing is it works fine in weston, so this seems to be a mutter issue after all.
I think this bacause of commit 24c3844 which now explicitly does a meta_wayland_keyboard_set_focus() when creating the xdg_popup. Not doing the meta_wayland_keyboard_set_focus() in meta_wayland_popup_create() fixes the issue with gtksourcecompletion widget.
=> moving to mutter
After discussing with Jonas on irc, we cannot change mutter as this would break the protocol xdg_shell v6 so another possibility is to either ignore the focu change in gtk+ or fix gtksourceview not to unmap the completion window on focus out event.
On further consideration (and tests!) it appears that ignoring the focus change event in gtk+/gtk wayland backend won't work reliably. Even though it will not hide the completion popup window anymore, the key press events won't have any effect as the actual window with keyboard focus is the popup window, and gtksourceview completion widget doesn't expect this. So I think this really needs to be addressed in the gtksourceview completion widget.
(In reply to Olivier Fourdan from comment #12) > On further consideration (and tests!) it appears that ignoring the focus > change event in gtk+/gtk wayland backend won't work reliably. > > Even though it will not hide the completion popup window anymore, the key > press events won't have any effect as the actual window with keyboard focus > is the popup window, and gtksourceview completion widget doesn't expect this. > > So I think this really needs to be addressed in the gtksourceview completion > widget. Did you try making sure the fake keyboard focus was the window of the keyboard events as well? I think you'd both need to fake the focus to be like X11, but also "route" the key events to the fake keyboard focus window.
FWIW, when ungrabbed (as it's the case), GTK+ expects key events to happen on the toplevel, which then propagates it to the focus widget specifically, so in addition to filtering focus events, key events should be delivered there. But then this shouldn't happen while grabbed, I guess gdk/wayland/ might be changed to check existing grabs on that device, and look up the toplevel or not depending on the situation. Although also think that gdk_windowing_got_event() does too little (i.e. nothing) to ensure key events are delivered to the right place according to GDK grabs, it just expects the events to be sent to the right window, that should probably change.
Actually, I am looking at killing two birds with one stone here, as gtksourceview doesn't implement the gdk_seat_grab() which causes a warning each time the completion windows shows up (or not, actually), having the grab as expected woult actually fix the warning and also work on Wayland.
Created attachment 337937 [details] [review] [PATCH] completion: Fix for xdg_shell v6 on Wayland xdg_popup in xdg_shell v6 on Wayland now takes focus (previously, the behaviour was undefined). Unfortunately, gtksourceview would hide the completion info window on focus change, meaning that the completion info window would never show up on Wayland, as it would get hidden as soon as shown due to the focus change... This patch does two things, first it implements the appropriate gdk_seat_grab() mechanism so that the completion info doesn't generate a gdk warning each time a completion window is shown, and second, it won't hide the completion info window on focus change as this is not necessary as we would now have a grab. This seems to work reasonably well on both X11 and Wayland and gets rid of the gdk warning about the missing grab.
Created attachment 337961 [details] [review] wayland: Allow grabless xdg_popups xdg_shell v6 allows grabless popups, whose behavior is not that different from override redirect windows with no grab to take keyboard input (and pointer events outside). This means we can relax the requirement to have a grab before creating an xdg_popup. The warning is still useful to have so people stop relying on gdk_window_show();gdk_device_grab() being an ok pattern to popup a window, it's been moved to wayland implementation of gdk_device_grab() instead, so we warn if trying to grab a GDK_WINDOW_TEMP window that's already visible.
Review of attachment 337961 [details] [review]: attachment 337961 [details] [review] looks good to me and was tested successfully earlier in the day.
Review of attachment 337961 [details] [review]: This looks good to me too, and was pretty much what the non-grabbing xdg_popup was meant for.
Review of attachment 337961 [details] [review]: I tested this successfully.
Review of attachment 337961 [details] [review]: Setting status back to "accepted-commit_now"
Created attachment 337998 [details] focus break log With this patch the popup indeed show in wayland, however I've been getting focus issues. In some circumstances it seems focus on the application window is completely lost. If I type fast and don't use the completion suggestions, sometimes the popup is dismissed (b/c there are no suggestions anymore?) and the main application (builder) loses focus completely. This is more game breaking than having no popup. Attached a wayland debug log where this behaviour is observed once (and I quit the application after). I can't pinpoint the exact occurrence however.
(In reply to Marinus Schraal from comment #22) > Created attachment 337998 [details] > focus break log > > With this patch the popup indeed show in wayland, however I've been getting > focus issues. In some circumstances it seems focus on the application window > is completely lost. > > If I type fast and don't use the completion suggestions, sometimes the popup > is dismissed (b/c there are no suggestions anymore?) and the main > application (builder) loses focus completely. This is more game breaking > than having no popup. > > Attached a wayland debug log where this behaviour is observed once (and I > quit the application after). I can't pinpoint the exact occurrence however. Yes, I can reproduce that, but this looks like an issue in mutter focus management rather than a bug in gtk+ xdg_popup code. Using MUTTER_VERBOSE shows the issue: STARTUP: Applying startup props to W5 id "(none)" STARTUP: Window W5 has _NET_WM_USER_TIME of 76886753 PLACEMENT: Putting window W5 on same workspace as parent W1 (lt-test-co) WINDOW_STATE: Putting W5 in the move_resize queue WINDOW_STATE: Putting W5 in the calc_showing queue PLACEMENT: Putting window W5 on active workspace STACK: Adding window W5 to the stack STACK: Window W5 has stack_position initialized to 1 STACK: Syncing window stack to server STACK: Adding 1 windows to sorted list STACK: Recomputing layers STACK: Window W5 on layer 7 type = 9 has_focus = 0 STACK: Window W5 moved from layer 8 to 7 STACK: Window W1 (lt-test-co) on layer 2 type = 0 has_focus = 1 STACK: Reapplying constraints STACK: Constraining W5 above W1 (lt-test-co) due to transiency STACK: W5 above at 1 > W1 (lt-test-co) below at 0 STACK: Sorting stack list STACK: Bottom to top: 2:0 - W1 (lt-test-co) 7:1 - W5 STACK: Restacking 2 windows (W5 is the popup, W1 the test-completion window from gtksourceview) [...] STARTUP: COMPARISON (continued): focus_window : W1 (lt-test-co) fw->net_wm_user_time_set : 1 fw->net_wm_user_time : 76886786 STARTUP: window W5 focus prevented by other activity; 76886753 < 76886786 WINDOW_STATE: Window W5 does not focus on map, and does not place on top on map. STARTUP: The focus window W1 (lt-test-co) is an ancestor of the newly mapped window W5 which isn't being focused. Unfocusing the ancestor. This is where things start to go wrong ^^^^ FOCUS: W1 (lt-test-co) is now the previous focus window due to being focused out or unmapped VERBOSE: Grabbing unfocused window buttons for W1 (lt-test-co) FOCUS: * Focus --> NULL with serial 500 STACK: Syncing window stack to server STACK: Bottom to top: 2:0 - W1 (lt-test-co) 7:1 - W5 STACK: Restacking 2 windows STACK: MetaStackTracker state xserver_serial: 339 verified_stack: 0x200013 0x200001 0x200002 0x200003 0x200004 0x200005 0x200006 0x200007 0x200008 0x20000d 0x20000e (mutter) 0x200012 0x200016 0x100000001 (lt-test-co) 0x100000005 unverified_predictions: [] FOCUS: Focus out event received on 0x270 (root window) mode NotifyNormal detail NotifyAncestor serial 500 FOCUS: Ignoring focus event generated by a grab or other weirdness mutter-Message: xdg_popup_destructor mutter-Message: xdg_surface_destroy mutter-Message: xdg_surface_destructor mutter-Message: xdg_popup_role_finalize mutter-Message: xdg_surface_role_finalize VERBOSE: Unmanaging W5 FOCUS: Unmanaging window W5 which doesn't currently have focus And from now on, the test-completion window has lost focus... So, IMHO, we should clone this bug for mutter and continue from there.
(In reply to Olivier Fourdan from comment #23) > So, IMHO, we should clone this bug for mutter and continue from there. This makes sense yes. Thinking of it, the keyboard focus semantics for non-grabbing zxdg_shell_v6 popups is pretty undefined. Same applies for subsurfaces, but in practice, subsurface never receive keyboard focus, so it might make sense to do the same for non-grabbing popups.
(In reply to Jonas Ådahl from comment #24) > (In reply to Olivier Fourdan from comment #23) > > So, IMHO, we should clone this bug for mutter and continue from there. > > This makes sense yes. Thinking of it, the keyboard focus semantics for > non-grabbing zxdg_shell_v6 popups is pretty undefined. Same applies for > subsurfaces, but in practice, subsurface never receive keyboard focus, so it > might make sense to do the same for non-grabbing popups. Done => cloned as bug 773210 with complete MUTTER_VERBOSE log attached
(In reply to Jonas Ådahl from comment #24) > (In reply to Olivier Fourdan from comment #23) > > So, IMHO, we should clone this bug for mutter and continue from there. > > This makes sense yes. Thinking of it, the keyboard focus semantics for > non-grabbing zxdg_shell_v6 popups is pretty undefined. Same applies for > subsurfaces, but in practice, subsurface never receive keyboard focus, so it > might make sense to do the same for non-grabbing popups. FWIW I agree with this. It makes sense to preserve the non-grabbing behavior across all capabilities, taking keyboard focus into it sounds like a partial grab of sorts.
Attachment 337961 [details] pushed as 2dfaae6 - wayland: Allow grabless xdg_popups