GNOME Bugzilla – Bug 731998
Can't close tab with X after dnd onto tab bar
Last modified: 2021-06-10 20:48:03 UTC
Drag a tab from one gnome-terminal window to another, and drop it on the tab bar (not on the terminal area). Then click on X. The tab is not closed, instead, this is printed: ** (gnome-terminal-server:17420): WARNING **: (terminal-notebook.c:113):terminal_notebook_remove_screen: runtime check failed: (gtk_widget_is_ancestor (GTK_WIDGET (screen), GTK_WIDGET (notebook))) (gnome-terminal-server:17420): Gtk-CRITICAL **: gtk_container_remove: assertion 'gtk_widget_get_parent (widget) == GTK_WIDGET (container) || GTK_IS_ASSISTANT (container)' failed The tab can be closed by any other means, just not the X button. Reminds me to bug 730389 comment 15 and onwards... but this bug predates that patch.
terminal_notebook_add_screen() says: g_signal_connect (tab_label, "close-button-clicked", G_CALLBACK (close_button_clicked_cb), notebook); This is where we remember to which notebook that close button belongs. When drag-n-dropped on another tab bar, the close button remains the same. MDI is apparently not notified of this action at all (none of the terminal_mdi_container_* methods are called), and in turn terminal_notebook_{add,remove}_screen are not called either. So, the screen-close-request signal is emitted on the wrong notebook. Note: terminal_notebook_page_{added,removed} (these are methods directly on the terminal class, without MDI) are called properly, and take care of updating the Tabs menu and such. I *guess* these are the only entry points where we get notified by Gtk+ about a DND onto the tab bar, so we should somehow propagate this to the MDI??? ChPe? Heeeeelp!!! :)
Created attachment 278894 [details] [review] Fix (workaround) How about this fix/workaround? I've modified the close button callback to look up the notebook when it's executed. This doesn't address the core problem and looks fragile against future changes, but fixes the current problem without heavy refactoring. Maybe good enough for 3-12 while a more proper fix is created for master?
Review of attachment 278894 [details] [review]: Add a comment in the code documenting why it's not using a notebook as user-data ? ::: src/terminal-notebook.c @@ +64,3 @@ TerminalScreen *screen; + notebook = TERMINAL_NOTEBOOK (gtk_widget_get_parent (GTK_WIDGET (tab_label))); Maybe use notebook = gtk_widget_get_ancestor (GTK_WIDGET (screen), TERMINAL_TYPE_NOTEBOOK); and guard against it being NULL (can the close button be clicked while it's in a DND operation?).
Review of attachment 278894 [details] [review]: ::: src/terminal-notebook.c @@ +64,3 @@ TerminalScreen *screen; + notebook = TERMINAL_NOTEBOOK (gtk_widget_get_parent (GTK_WIDGET (tab_label))); Done. Not sure if it can be NULL, but you just gave an excellent idea to find a new segfault :/
Created attachment 278924 [details] [review] Fix (workaround) v2
Comment on attachment 278924 [details] [review] Fix (workaround) v2 3-12 too, if you want.
Comment on attachment 278924 [details] [review] Fix (workaround) v2 Committed. I guess I'd leave the bug open. I think MDI's data about which tab is where is still not properly updated and this might lead to other bugs. Eventually we should fix the underlying cause, not the symptom.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/7467.