wmww / gtk-layer-shell

A library to create panels and other desktop components for Wayland using the Layer Shell protocol
GNU General Public License v3.0
319 stars 16 forks source link

Closing a menu where an item displays a tooltip from a layer-shell window causes a protocol error #178

Closed Tamaranch closed 1 month ago

Tamaranch commented 6 months ago

If the window is not layer-shell, GTK displays a warning, but this does not cause a protocol error.

Code to reproduce the issue:

static gboolean
press (GtkWidget *widget,
       GdkEventButton *event,
       gpointer data)
{
  gtk_menu_popup_at_pointer (data, (GdkEvent *) event);
  return FALSE;
}

gint main (gint argc, gchar **argv)
{
  gtk_init (&argc, &argv);

  GtkWidget *menu = gtk_menu_new ();
  GtkWidget *item = gtk_menu_item_new_with_label ("item1");
  gtk_widget_set_tooltip_text (item, "tooltip1");
  gtk_menu_shell_append (GTK_MENU_SHELL (menu), item);
  item = gtk_menu_item_new_with_label ("item2");
  gtk_widget_set_tooltip_text (item, "tooltip2");
  gtk_menu_shell_append (GTK_MENU_SHELL (menu), item);
  item = gtk_menu_item_new_with_label ("item3");
  gtk_widget_set_tooltip_text (item, "tooltip3");
  gtk_menu_shell_append (GTK_MENU_SHELL (menu), item);
  gtk_widget_show_all (menu);
  gtk_widget_hide (menu);

  GtkWidget *window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
  GtkWidget *widget = gtk_button_new_with_label ("press me");
  gtk_layer_init_for_window (GTK_WINDOW (window));
  gtk_layer_set_keyboard_mode (GTK_WINDOW (window), GTK_LAYER_SHELL_KEYBOARD_MODE_ON_DEMAND);
  gtk_container_add (GTK_CONTAINER (window), widget);
  g_signal_connect (widget, "button-press-event", G_CALLBACK (press), menu);
  gtk_widget_show_all (window);

  GMainLoop *loop = g_main_loop_new (NULL, FALSE);
  g_signal_connect_swapped (window, "destroy", G_CALLBACK (g_main_loop_quit), loop);
  g_main_loop_run (loop);
  g_main_loop_unref (loop);
}

Steps to reproduce the issue:

I get these logs in labwc, but it crashes with any compositor:

[2714599.198] wl_keyboard@26.key(848, 36449698, 1, 1)
[2714613.766]  -> xdg_popup@39.destroy()
[2714613.883]  -> xdg_surface@38.destroy()
[2714613.940]  -> wl_surface@36.destroy()
[2714613.995]  -> wl_buffer@41.destroy()
[2714614.037]  -> wl_shm_pool@37.destroy()
[2714614.253]  -> wl_pointer@19.set_cursor(846, wl_surface@24, 7, 4)
[2714614.313]  -> wl_surface@24.attach(wl_buffer@35, 0, 0)
[2714614.358]  -> wl_surface@24.set_buffer_scale(1)
[2714614.399]  -> wl_surface@24.damage(0, 0, 24, 24)
[2714614.444]  -> wl_surface@24.commit()
[2714614.573]  -> xdg_popup@42.destroy()
[2714614.618]  -> xdg_surface@43.destroy()
[2714614.673]  -> wl_surface@40.destroy()
[2714614.723]  -> wl_buffer@45.destroy()
[2714614.762]  -> wl_shm_pool@44.destroy()
[2714615.250]  -> wl_surface@34.destroy()
[2714619.765] wl_display@1.error(xdg_wm_base@29, 2, "xdg_popup was destroyed while it was not the topmost popup")
Gdk-Message: 18:37:48.286: Error 71 (Erreur de protocole) dispatching to Wayland display.

If you comment out the layer-shell part in the code above, it doesn't crash and here are the logs:

[2781397.776] wl_keyboard@26.key(1037, 36516497, 1, 1)

(q:204306): Gdk-WARNING **: 18:38:55.075: Tried to unmap the parent of a popup
[2781409.513]  -> xdg_popup@43.destroy()
[2781409.541]  -> xdg_surface@42.destroy()
[2781410.116]  -> wl_surface@40.destroy()
[2781410.156]  -> wl_buffer@44.destroy()
[2781410.181]  -> wl_shm_pool@41.destroy()
[2781410.275]  -> xdg_popup@38.destroy()
[2781410.303]  -> xdg_surface@37.destroy()
[2781410.332]  -> wl_surface@31.destroy()
[2781410.360]  -> wl_buffer@39.destroy()
[2781410.383]  -> wl_shm_pool@32.destroy()
[2781410.792]  -> wl_pointer@19.set_cursor(1035, wl_surface@24, 7, 4)
[2781410.848]  -> wl_surface@24.attach(wl_buffer@36, 0, 0)
[2781410.866]  -> wl_surface@24.set_buffer_scale(1)
[2781410.904]  -> wl_surface@24.damage(0, 0, 24, 24)
[2781410.929]  -> wl_surface@24.commit()
[2781411.824]  -> wl_surface@35.destroy()

gtk-layer-shell 0.8.2 gtk 3.24.41

wmww commented 1 month ago

Oh! this is an interesting one. Thanks for the nice simple reproducer, and sorry it took me so long to get to.

Normally GTK makes sure popups are unmapped before parents. However we need to delete our Wayland objects before the GTK unmap handler runs (or else we get a different protocol error), so we override it to do our stuff first. This causes us to unmap parents before children, causing this problem.

There's not an obvious way to let GTK handle unmapping children, so instead I opted to keep a list of children and handle unmapping myself. This adds a little complexity but seems to work.

With some improvements to the integration test system I managed to add a test for this bug, so that should keep it from popping up again.

Tamaranch commented 1 month ago

Thanks, I can confirm it also fixes the issue in my real use case (app menu in xfce4-panel).