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