Bug 439159 - "Add widgets..." and "Show alternatives..." options from context menu stop working after manually restarting plasmashell process using `plasmashell --replace`
Summary: "Add widgets..." and "Show alternatives..." options from context menu stop wo...
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: general (show other bugs)
Version: 5.23.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: 1.0
Assignee: David Edmundson
URL:
Keywords:
: 443647 445438 445796 456556 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-06-25 16:14 UTC by Patrick Silva
Modified: 2022-11-12 15:57 UTC (History)
14 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.26.4
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2021-06-25 16:14:26 UTC
STEPS TO REPRODUCE
1. run "plasmashell --replace" with krunner
2. right-click on Plasma panel
3. choose "Add widgets..." or "Show alternatives..."

OBSERVED RESULT
nothing happens

EXPECTED RESULT
the option chosen in the last step works

SOFTWARE/OS VERSIONS
Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.22.80
KDE Frameworks Version: 5.84.0
Qt Version: 5.15.3
Graphics Platform: X11
Comment 1 Nate Graham 2021-07-29 15:54:29 UTC
Can reproduce.
Comment 2 Alexander Potashev 2021-09-03 14:59:39 UTC
Same here on Plasma 5.21.5, KF 5.83.0, Qt 5.15.2, X11.
Comment 3 Nate Graham 2021-09-29 18:09:56 UTC
Basically the first time Plasma crashes or is manually restarted during development activities, these functions break.

I've done some debugging and it appears that the problem is ShellCorona::handleContainmentAdded() not being run after plasmashell is restarted. This function is solely responsible for handling the signals that open those popups and handle the initial plasmoid setup script (if present), so when it doesn't get run, the connections don't get made and that functionality breaks.
Comment 4 Nate Graham 2021-09-29 18:41:01 UTC
It appears that configDialog.containmentPlugin isn't set by the time the panel configuration window appears...

Getting towards the end of my ability to debug this, I fear.
Comment 5 Nate Graham 2021-09-29 18:49:47 UTC
Actually the same bug happens when you try to trigger the Widget Explorer from the context menu, so I guess it isn't getting set in multiple places, or else maybe the need for it to be set is unnecessary?
Comment 6 Nate Graham 2021-09-29 19:29:12 UTC
Urgh the complexity of this code makes me want to cry
Comment 7 Nate Graham 2021-09-29 19:49:06 UTC
Breakthrough thanks to rubber ducking with my son: restarting with plasmashell --replace causes this bug, but manually killing plasmashell and launching it manually with either `plasmashell` or `kstart5 plasmashell` avoids the bug. So there is something about plasmashell's --replace argument that triggers it...
Comment 8 Nate Graham 2021-09-29 19:52:47 UTC
It appears that this connection in shellcorona.cpp:

connect(this, &ShellCorona::containmentAdded, this, &ShellCorona::handleContainmentAdded);

happens too late when using plasmashell --restart. In this case, the containmentAdded() signal in corona.cpp is emitted, but it doesn't do anything because the connection hasn't been made yet. When launching plasmashell but it's not already running, the signal gets emitted after the connection has been made, so it always works.
Comment 9 Nate Graham 2021-09-29 22:35:30 UTC
The only difference seems to be this line in shell/main.cpp:

KDBusService service(KDBusService::Unique | KDBusService::StartupOption(replace ? KDBusService::Replace : 0));
Comment 10 Nate Graham 2021-09-29 22:41:29 UTC
Hmm maybe the problem is in KDbusAddons? This doesn't look right...


if (options & KDBusService::Replace) {
    //stuff
} else if (options & KDBusService::Unique) {


If I'm reading this right, when both KDBusService::Replace and KDBusService::Unique are specified, it seems like the code to handle KDBusService::Unique gets ignored.
Comment 11 Nate Graham 2021-09-29 23:41:04 UTC
Hmm, changing that doesn't seem to fix the problem...
Comment 12 Nate Graham 2021-09-30 00:25:10 UTC
Giving up for now since this only affects developer/power user setups and has a trivial workaround (`killall plasmashell && kstart5 plasmashell`)
Comment 13 David Edmundson 2021-10-04 16:08:09 UTC
You found all the parts.

Cold case:
 - we call ShellCorona::load from startup
 - we have no kactivities loaded yet, we do an early return and connect
 - we call ShellCorona::init
 - activities load, we start the real load via the connect

Restart case:
 - we call ShellCorona::load
 - we have no kactivities loaded yet, we do an early return and connect
 - We start an event loop, waiting for our bus name to appear
 - activities load, we start the real load
 - we call ShellCorona::init


>Urgh the complexity of this code makes me want to cry

Was also a valid analysis.
Comment 14 Patrick Silva 2021-10-13 02:35:09 UTC
*** Bug 443647 has been marked as a duplicate of this bug. ***
Comment 15 Patrick Silva 2021-11-14 01:28:22 UTC
*** Bug 445438 has been marked as a duplicate of this bug. ***
Comment 16 Patrick Silva 2021-11-20 16:22:50 UTC
*** Bug 445796 has been marked as a duplicate of this bug. ***
Comment 17 Waqar Ahmed 2022-01-19 06:43:48 UTC
I ran into this problem today but with the difference that "Show Alternatives.." is working for me but "Add Widgets" doesn't. Also, I am running plasma like normal, didn't restart it manually or anything so I think this is a non poweruser / developer bug.

Plasma version 5.23.4
Frameworks: 5.89
Comment 18 Nate Graham 2022-02-22 19:43:41 UTC
Raising priority as it seems to be affecting users after Plasma crashes, not just developers.
Comment 19 Fushan Wen 2022-06-15 01:36:02 UTC
Not a regression in 5.25.
Comment 20 Nate Graham 2022-07-13 19:16:45 UTC
*** Bug 456556 has been marked as a duplicate of this bug. ***
Comment 21 Bug Janitor Service 2022-09-16 13:51:10 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/2127
Comment 22 David Edmundson 2022-09-20 20:51:57 UTC
Git commit d79a927a4c7a8b489b985a6ae94bd9d8f082db93 by David Edmundson.
Committed on 20/09/2022 at 20:43.
Pushed by davidedmundson into branch 'master'.

[shell] Always call load after init

Currently we get two different startup paths:

Cold case:
 - we call ShellCorona::load from startup
 - we have no kactivities loaded yet, we do an early return and connect
 - we call ShellCorona::init
 - activities load, we start the real load via the connect

Restart case:
 - we call ShellCorona::load
 - we have no kactivities loaded yet, we do an early return and connect
 - We start an event loop, waiting for our bus name to appear
 - activities load, we start the real load
 - we call ShellCorona::init

Calling load before init results in broken behaviour as things aren't
initialized.

setShell is only called once in main.cpp really early on. It's not used
elsewhere or exposed on DBus. There's no need for it to call load, when
it will fail and wait for an event loop in almost all cases. Calling it
from init makes ordering easier to follow and consistent through all
paths.

M  +43   -28   shell/shellcorona.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/d79a927a4c7a8b489b985a6ae94bd9d8f082db93
Comment 23 Fushan Wen 2022-09-21 00:06:31 UTC
Git commit d2b4d751e7723f40c14b0dea676ab5426d77340d by Fushan Wen.
Committed on 21/09/2022 at 00:05.
Pushed by fusionfuture into branch 'master'.

shell: revert unrelated changes in d79a927a4c7a8b489b985a6ae94bd9d8f082db93

M  +1    -21   shell/shellcorona.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/d2b4d751e7723f40c14b0dea676ab5426d77340d
Comment 24 Devin Lin 2022-11-12 15:27:46 UTC
Git commit 8056e09e08b37e19b1f55179fc8bfa960eea8e24 by Devin Lin, on behalf of David Edmundson.
Committed on 12/11/2022 at 15:23.
Pushed by devinlin into branch 'Plasma/5.26'.

[shell] Always call load after init

Currently we get two different startup paths:

Cold case:
 - we call ShellCorona::load from startup
 - we have no kactivities loaded yet, we do an early return and connect
 - we call ShellCorona::init
 - activities load, we start the real load via the connect

Restart case:
 - we call ShellCorona::load
 - we have no kactivities loaded yet, we do an early return and connect
 - We start an event loop, waiting for our bus name to appear
 - activities load, we start the real load
 - we call ShellCorona::init

Calling load before init results in broken behaviour as things aren't
initialized.

setShell is only called once in main.cpp really early on. It's not used
elsewhere or exposed on DBus. There's no need for it to call load, when
it will fail and wait for an event loop in almost all cases. Calling it
from init makes ordering easier to follow and consistent through all
paths.

M  +43   -28   shell/shellcorona.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/8056e09e08b37e19b1f55179fc8bfa960eea8e24
Comment 25 Devin Lin 2022-11-12 15:27:54 UTC
Git commit a77f1bda64114712603a8c25bc2c678dc6957676 by Devin Lin, on behalf of Fushan Wen.
Committed on 12/11/2022 at 15:23.
Pushed by devinlin into branch 'Plasma/5.26'.

shell: revert unrelated changes in d79a927a4c7a8b489b985a6ae94bd9d8f082db93

M  +1    -21   shell/shellcorona.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/a77f1bda64114712603a8c25bc2c678dc6957676