SUMMARY When clicking on the update notifier in the system tray, Discover doesn't become frontmost if Discover is already open on the desktop but hidden behind other windows. This is confusing, as it appears that nothing happens. STEPS TO REPRODUCE 1. I see that the update notifier is present so I click on it. Discover launches. 2. I do other work and the Discover window gets buried. I forget about it. 3. I click on the update notifier and nothing happens. Discover happens to be buried under other windows but it doesn't become frontmost. So it seems something is broken. EXPECTED BEHAVIOUR Discover should become frontmost if it's already running and I click on the update notifier. SOFTWARE/OS VERSIONS Operating System: KDE neon 5.19 KDE Plasma Version: 5.19.2 KDE Frameworks Version: 5.71.0 Qt Version: 5.14.2 Kernel Version: 5.3.0-62-generic OS Type: 64-bit Processors: 4 × Intel® Core™ i5-2467M CPU @ 1.60GHz Memory: 7.7 GiB of RAM Graphics Processor: Mesa DRI Intel® Sandybridge Mobile
When clicked, Discover's notifier runs an ApplicationLauncherJob: auto *job = new KIO::ApplicationLauncherJob(KService::serviceByDesktopName(QStringLiteral("org.kde.discover"))); job->setUiDelegate(new KNotificationJobUiDelegate(KJobUiDelegate::AutoErrorHandlingEnabled)); job->start(); Clicking on it when Discover is already open causes Discover's Task Manager entry to turn orange, so clearly the job or something underneath it knows that there's an existing instance already open. I wonder if this is a case of KWin inappropriately preventing the application from coming to the front.
Are you using wayland? On wayland we don't have a mechanism to raise windows yet. That's a known and important issue in KWin.
No, on X11. I know this feature isn't available yet on Wayland.
It should be working in X11. Could you see that this line gets called? mainWindow->rootObject()->raise(); This code hasn't changed over the years, so it's possible that it's a KWin issue.
Yep, it does. I guess this is a KWin regression then. Moving over there for now. Of note, if I disable focus stealing prevention, everything works properly. So I guess KWin is mis-characterizing this action as focus stealing, which is not accurate since it's an explicitly user-initiated action.
...Where by "disable focus stealing prevention" I mean "change focus stealing prevention from Low to None in the Window Behavior KCM".
There has been one potentially relevant change in kwin wrt focus. But there clearly have also been changes on the discover side as ApplicationLaunchJob didn't exist a few months ago. If it doesn't pass focus correctly then kwin could still well be doing the right thing.
The potentially relevant kwin commit: a0c4a8e766a2160213838daf6f71c7ae6c3705df
Reverting a0c4a8e766a2160213838daf6f71c7ae6c3705df has no effect for me, BTW.
Then I very much doubt it's kwin at fault.
So then the problem is in ApplicationLauncherJob or Discover? What exactly is the problem? Why does KWin think focus is being stolen?
*** Bug 426632 has been marked as a duplicate of this bug. ***
Seems like the bug is somewhere in ApplicationLauncherJob or KWin; either the job isn't marking the request correctly so KWin doesn't know that it's not focus stealing, or else it's doing stuff right but KWin does the wrong thing anyway. Don't know which one yet.
The problem is that discover doesn't provide any startup id. I doubt it's a kwin bug.
Created attachment 131824 [details] PoC Could somebody please verify that this patch fixes the bug? Note that it's just a poc patch.
I was debugging something similar elsewhere (for something else) There is a bug is the KIO side, but in the part that's now disabled again. Bug is: KStartupInfo::setupStartupEnv does "qputenv" this updates the env of the application about to launch. later we call systemdprocessrunner.cpp { QStringLiteral("Environment"), m_process->environment() }, but m_process->environment() is just an empty array. There's some QProcess code to say if it's empty we take the system env, which is why it normally works, but that doesnt' help here. We need to refactor the startup env side a bit.
> but m_process->environment() is just an empty array. as expected, KProcessRunner needs to call QProcess::setProcessEnvironment(), doesn't it?
Yes, but ideally we want that to only include the environment values that are worth exporting, not the entire thing that process currently has.
(In reply to Vlad Zahorodnii from comment #15) > Created attachment 131824 [details] > PoC > > Could somebody please verify that this patch fixes the bug? Note that it's > just a poc patch. Doesn't seem to help, sorry.
@Nate maybe you could try Vlad's patch but adding the setter on the ::showDiscoverUpdates() method instead of ::showDiscover()?
No, that doesn't change anything for me.
*** Bug 429573 has been marked as a duplicate of this bug. ***
*** Bug 430561 has been marked as a duplicate of this bug. ***
*** Bug 430547 has been marked as a duplicate of this bug. ***
Dupes piling up; marking as VHI.
Nothing indicates this is kwin at fault, and nothing indicates ApplicationLauncherJob is at fault (https://invent.kde.org/frameworks/kio/-/merge_requests/298) KDBusService still has multiple things wrong.
I have some working code: https://phabricator.kde.org/P655 in discover https://phabricator.kde.org/P656 in KDBusService With that, startupID is passed correctly.
Thanks David! Note that we have duplicate bug reports for the same issue in Elisa, Konversation, and the Emoji picker. Will the KDBusService patch fix all of them, or will they each need app-specific fixes as well, like Discover apparently does?
Note that I cannot merge that KDBusService patch as-is. I can't depend on KWindowsystem, but somehow we need to relay that startup id value to the outside. The rest of those clients do need to do something in their handling of KDBusService::activate In fairness, the docs for KDBusService to specify this, so hopefully clients should be following that.
A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/61
Git commit 57dc3b725d1bfed713dffbc13543f79300503d61 by David Edmundson. Committed on 04/01/2021 at 15:58. Pushed by ngraham into branch 'master'. Set startupID before raising Otherwise it will be blocked correctly by window manager focus policies M +1 -1 CMakeLists.txt M +1 -0 discover/CMakeLists.txt M +12 -1 discover/main.cpp https://invent.kde.org/plasma/discover/commit/57dc3b725d1bfed713dffbc13543f79300503d61