search

p-e-w / argos

Create GNOME Shell extensions in seconds
https://extensions.gnome.org/extension/1176/argos/
1.66k stars 112 forks source link

remove redeclaration of _onDestroy() #128

Closed Valent-in closed 1 year ago

Valent-in commented 3 years ago

_onDestroy() is already present in panelMenu.Button.

https://github.com/GNOME/gnome-shell/blob/e6089c83e2993098814b7056e1c7f54e5dc06820/js/ui/panelMenu.js#L187

So this function is called twice. Put log("### something") into it to check. And proper Button._onDestroy() is not called at all. This leads to shell crash on screen lock (issue #79).

removed this.menu.removeAll() - fix errors in journal because menu is already destroyed.

mwilck commented 1 year ago

I can't reproduce this here. I see no crashes when locking the screen (or disabling the extension), and the function is not called twice (note that if you have several buttons, _onDestroy() will be called for each).

Valent-in commented 1 year ago

Probably additional checks has been added to ST.

Anyway this is not ok to override this function without super._onDestroy()

and the function is not called twice

For me it does (Ubuntu 22.04, Gnome 42.5). I used journalctl -f -o cat /usr/bin/gnome-shell It is called once if this line commented: https://github.com/p-e-w/argos/blob/e2d68ea23eed081fccaec06c384e2c5d2acb5b6b/argos%40pew.worldwidemann.com/button.js#L46

Valent-in commented 1 year ago

I think super._onDestroy() should be added in the end of _onDestroy() and line 46 removed.

But I deleted my fork and do not have local copy, so can not update commit.

mwilck commented 1 year ago

Anyway this is not ok to override this function without super._onDestroy()

I thought so, too, and experimented a bit with various invocations of super._onDestroy(), but it just got really bad when I tried.