After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 771694 - GtkSourceView completion popup window not shown, no grabbed seat found
GtkSourceView completion popup window not shown, no grabbed seat found
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: WaylandRelated 772360 773000 773136 773210
 
 
Reported: 2016-09-20 08:42 UTC by Marinus Schraal
Modified: 2016-10-20 09:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
WAYLAND_DEBUG=1 gnome-builder (426.08 KB, text/plain)
2016-09-20 08:42 UTC, Marinus Schraal
  Details
[PATCH] completion: Fix for xdg_shell v6 on Wayland (5.60 KB, patch)
2016-10-18 11:48 UTC, Olivier Fourdan
none Details | Review
wayland: Allow grabless xdg_popups (3.74 KB, patch)
2016-10-18 18:18 UTC, Carlos Garnacho
committed Details | Review
focus break log (550.47 KB, text/plain)
2016-10-19 08:32 UTC, Marinus Schraal
  Details

Description Marinus Schraal 2016-09-20 08:42:50 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".
Comment 1 Sébastien Wilmet 2016-09-25 09:29:56 UTC
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.
Comment 2 Christian Hergert 2016-09-25 20:25:32 UTC
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.
Comment 3 Sébastien Wilmet 2016-10-01 11:13:55 UTC
(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?
Comment 4 Marinus Schraal 2016-10-02 12:23:53 UTC
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.
Comment 5 Sébastien Wilmet 2016-10-09 11:26:15 UTC
Moving from GtkSourceView to GTK+, because it is an API break.
Comment 6 Olivier Fourdan 2016-10-12 13:35:22 UTC
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.
Comment 7 Olivier Fourdan 2016-10-12 15:02:35 UTC
Ah no sorry, I take that back... gtk+ withdraw the popup
Comment 8 Olivier Fourdan 2016-10-17 09:47:56 UTC
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.
Comment 9 Olivier Fourdan 2016-10-17 12:33:23 UTC
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.
Comment 10 Olivier Fourdan 2016-10-17 12:33:56 UTC
=> moving to mutter
Comment 11 Olivier Fourdan 2016-10-18 07:39:43 UTC
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.
Comment 12 Olivier Fourdan 2016-10-18 08:44:16 UTC
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.
Comment 13 Jonas Ådahl 2016-10-18 08:51:24 UTC
(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.
Comment 14 Carlos Garnacho 2016-10-18 09:40:07 UTC
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.
Comment 15 Olivier Fourdan 2016-10-18 11:46:44 UTC
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.
Comment 16 Olivier Fourdan 2016-10-18 11:48:50 UTC
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.
Comment 17 Carlos Garnacho 2016-10-18 18:18:23 UTC
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.
Comment 18 Olivier Fourdan 2016-10-18 19:26:29 UTC
Review of attachment 337961 [details] [review]:

attachment 337961 [details] [review] looks good to me and was tested successfully earlier in the day.
Comment 19 Jonas Ådahl 2016-10-19 02:35:48 UTC
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.
Comment 20 Daniel Buch 2016-10-19 05:51:30 UTC
Review of attachment 337961 [details] [review]:

I tested this successfully.
Comment 21 Olivier Fourdan 2016-10-19 06:26:21 UTC
Review of attachment 337961 [details] [review]:

Setting status back to "accepted-commit_now"
Comment 22 Marinus Schraal 2016-10-19 08:32:55 UTC
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.
Comment 23 Olivier Fourdan 2016-10-19 09:10:37 UTC
(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.
Comment 24 Jonas Ådahl 2016-10-19 09:17:33 UTC
(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.
Comment 25 Olivier Fourdan 2016-10-19 09:25:26 UTC
(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
Comment 26 Carlos Garnacho 2016-10-20 09:31:10 UTC
(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.
Comment 27 Carlos Garnacho 2016-10-20 09:40:04 UTC
Attachment 337961 [details] pushed as 2dfaae6 - wayland: Allow grabless xdg_popups