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 768017 - [Wayland] menus opened from access keys (mnemonic menu items) hide instantly
[Wayland] menus opened from access keys (mnemonic menu items) hide instantly
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.20.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2016-06-24 19:05 UTC by Christian Stadelmann
Modified: 2016-08-20 03:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] wayland: Use keyboard serial for implicit grab (2.62 KB, patch)
2016-06-30 11:04 UTC, Olivier Fourdan
none Details | Review
[PATCH] wayland: Use keyboard serial for implicit grab (2.41 KB, patch)
2016-06-30 11:07 UTC, Olivier Fourdan
none Details | Review
[PATCH] wayland: Use keyboard serial for implicit grab (2.42 KB, patch)
2016-06-30 14:00 UTC, Olivier Fourdan
committed Details | Review

Description Christian Stadelmann 2016-06-24 19:05:37 UTC
Steps to reproduce:
0. have a wayland compositor, e.g. weston or gnome-wayland session
1. open any Gtk+ 3.x application with "old" menus (see "Affected applications" below)
2. use any mnemonic to open a menu

What happens:
Menu shows with animation. Once the animation is finished (<1 second) the menu hides.

What should happen:
menu should be shown until it loses focus or [Esc] is pressed

Affected applications:
* gnome-terminal
* evolution
* anjuta
* libreoffice (recent Gtk+ 3 builds, default on Fedora 24)
* probably others

This issue does not affect these types of applications:
* Qt 4 or 5 based applications
* Gtk+ 2 based applications
* firefox (drawing its own menu bar)
* Gtk+ 3 based applications with menus based on GtkPopover (e.g. epiphany, gedit)
* Gtk+ 3 based applications with X11 backend (even on gnome-wayland sessions)

This issue is also present when running Gtk+ 3 applications inside weston, but it is specific to the wayland backend.

This issue is present whether or not animations are enabled. With animations disabled you can barely see that the menu was opened.

Workaround:
Start applications with GDK_BACKEND=x11 added to environment when running a gnome-wayland session or run X11 sessions.
Comment 1 Matthias Clasen 2016-06-24 21:05:08 UTC
Does this only happen if the window doesn't have a keyboard focus widget ?
Comment 2 Christian Stadelmann 2016-06-24 21:42:10 UTC
(In reply to Matthias Clasen from comment #1)
> Does this only happen if the window doesn't have a keyboard focus widget ?

I don't know what a keyboard focus widget is. How can I find out?
Comment 3 Christian Stadelmann 2016-06-24 21:44:29 UTC
It also happens to gtk3-demo-application and gtk3-demo, section "Menus".
Comment 4 Matthias Clasen 2016-06-25 12:19:32 UTC
but only until you click in the window to move the focus in the text view, right ?
Comment 5 Christian Stadelmann 2016-06-26 21:00:13 UTC
Yes, after focusing any text view, this issue is gone.
Comment 6 Olivier Fourdan 2016-06-30 08:28:15 UTC
Looking at wayland debug logs and gtk+ code, we get an xdg_popup_done as soon as the menu is mapped on screen, and that hides the popup window, thus triggering a withdrawn state and consequently a menu popdown.
Comment 7 Olivier Fourdan 2016-06-30 08:33:02 UTC
The popup gets dismissed by the compositor because the serial is 0.
Comment 8 Olivier Fourdan 2016-06-30 08:44:14 UTC
Right, so the serial is 0 because _gdk_wayland_seat_get_last_implicit_grab_serial() returns 0 and it returns 0 because wayland_seat->pointer_info.press_serial is 0

That explains why as soon as the user clicks on the window, we update the pointer_info.press_serial and the serial is updated (and not 0 anymore).
Comment 9 Olivier Fourdan 2016-06-30 11:04:02 UTC
Created attachment 330645 [details] [review]
[PATCH] wayland: Use keyboard serial for implicit grab

An xdg-popup requires a serial that the compositor will compare against
its own serial and will dismiss the popup if it doesn't match.

gtk+ uses either a pointer or touch serial for its helper function
_gdk_wayland_seat_get_last_implicit_grab_serial() but if the menu is
triggered before the user has had any pointer or touch interaction with
the client, using a keyboard shortcut, there is neither pointer nor
touch serial available, and gtk+ will use 0 as the default.

As a result, the compositor will instantly dismiss the xdg-popup. In
this case, gtk+ should use the keyboard serial instead.

Track keyboard serial as well and use the keyboard serial as the value
if there is no newer pointer or touch serial available.
Comment 10 Olivier Fourdan 2016-06-30 11:05:21 UTC
Note: attachment 330645 [details] [review] will not work with weston because xdg-shell support in weston is broken for xdg-popup, it doesn't check against keyboard serial (I shall post a patch for weston as well).

But it works with gnome-shell/mutter where xdg-shell support is correct.
Comment 11 Olivier Fourdan 2016-06-30 11:07:38 UTC
Created attachment 330646 [details] [review]
[PATCH] wayland: Use keyboard serial for implicit grab

v2: sorry, forgot to remove a debugging trace before uploading...
Comment 12 Jonas Ådahl 2016-06-30 12:24:08 UTC
Review of attachment 330646 [details] [review]:

::: gdk/wayland/gdkdevice-wayland.c
@@ +185,3 @@
   GSettings *keyboard_settings;
   uint32_t keyboard_time;
+  uint32_t keyboard_serial;

This should probably be named "keyboard_key_serial" or something, since there are more serials (wl_keyboard.enter) that we won't store here.
Comment 13 Olivier Fourdan 2016-06-30 14:00:08 UTC
Created attachment 330658 [details] [review]
[PATCH] wayland: Use keyboard serial for implicit grab

(In reply to Jonas Ådahl from comment #12)
> Review of attachment 330646 [details] [review] [review]:
> 
> ::: gdk/wayland/gdkdevice-wayland.c
> @@ +185,3 @@
>    GSettings *keyboard_settings;
>    uint32_t keyboard_time;
> +  uint32_t keyboard_serial;
> 
> This should probably be named "keyboard_key_serial" or something, since
> there are more serials (wl_keyboard.enter) that we won't store here.

Sure, patch updated.
Comment 14 Olivier Fourdan 2016-06-30 14:03:54 UTC
For reference, weston patches submitted here: https://patchwork.freedesktop.org/series/9321/
Comment 15 Olivier Fourdan 2016-08-19 07:50:04 UTC
Anyone to do a review of attachment 330658 [details] [review], it looks it might have been forgotten...
Comment 16 Jonas Ådahl 2016-08-19 07:54:43 UTC
Review of attachment 330658 [details] [review]:

(In reply to Olivier Fourdan from comment #15)
> Anyone to do a review of attachment 330658 [details] [review] [review], it looks it
> might have been forgotten...

It indeed seems so. Patch looks good to me.