GNOME Bugzilla – Bug 757556
AppIconMenu not properly closed on source actor destroy
Last modified: 2021-07-05 14:39:14 UTC
Created attachment 314778 [details] [review] AppIconMenu: properly destroy on source destroy. If the appIcon is destroyed while the popupMenu is shown -- this can happen if a non favorite application was closing or crashes -- the menu actor is improperly destroyed, resulting in the attemp to ungrab a now removed actor. (gnome-shell:12855): St-CRITICAL **: st_widget_get_theme_node called on the widget [0x6bebbe0 StBin.popup-menu-boxpointer popup-menu app-well-menu] which is not in the stage. (gnome-shell:12855): Gjs-WARNING **: JS ERROR: Exception in callback for signal: open-state-changed: Error: incorrect pop The attached patch makes popupMenu properly close first, thus doing a clean ungrab.
Review of attachment 314778 [details] [review]: While we change it, it probably makes sense to move this bit into PopupMenu - it's most likely in the appIcon case, but destroying the menu when its source actor goes away generally looks like the right thing to do. ::: js/ui/appDisplay.js @@ +1815,3 @@ this.close(); })); + source.actor.connect('destroy', Lang.bind(this, function () { this.destroy(); })); Better written as: source.actor.connect('destroy', Lang.bind(this, this.destroy));
Created attachment 315085 [details] [review] AppIconMenu: properly destroy on source destroy. I clean up the path as suggested.
Review of attachment 315085 [details] [review]: I still think this would make sense generically in PopupMenu, but OK.
Created attachment 315097 [details] [review] Close and cleanup PopupMenu on actor destroy This tries to solve the issue at the popupMenu level, while still being compatible with the previously proposed patch. While the _onDestroy method is defined in the PopupMenuBase class, the wiring to the signal has to be done on the derived classes. It seems to me that only the popupMenu class requires this.
Review of attachment 315097 [details] [review]: Not sure what the purpose of this patch is - what I was suggesting was to move the code that connects to sourceActor::destroy from AppIconMenu to PopupMenu (simply because it seems a reasonable behavior, not because I really expect it to be an issue for other menus).
(In reply to Florian Müllner from comment #5) > Review of attachment 315097 [details] [review] [review]: > > Not sure what the purpose of this patch is The idea was that there should not be a PopupMenu::destroy public method which if called when the menu is not closed results in a bug.
Comment on attachment 315085 [details] [review] AppIconMenu: properly destroy on source destroy. Attachment 315085 [details] pushed as 1545596 - AppIconMenu: properly destroy on source destroy. Pushing the a-c-n patch.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/ Thank you for your understanding and your help.