[kio] Fix running konsole on Wayland
ClosedPublic

Authored by wbauer on Feb 25 2020, 3:22 PM.

Details

Summary

%i expands to "--icon someicon", but Qt recognizes this option only on X11/xcb.
So konsole failed to start on Wayland.

To fix this problem, pass "-qwindowicon" instead, which is supported on all platforms.

See https://community.kde.org/Guidelines_and_HOWTOs/Wayland_Porting_Notes#Exec_key_of_.desktop_files for details.

BUG: 408497
FIXED-IN: 5.68.0

Test Plan

Create a menu entry in kmenueditor that calls a command line application (say, "top"), with "Run in Terminal" (on the "Advanced" tab) enabled.
Try to start it from the application menu.

konsole starts successfully on Wayland now too, with or without an icon set.
Still works on X11/xcb as well.

Additional debug output confirms that konsole is called with "-qwindowicon someicon" now, but only if there actually is an icon set in the .desktop file.

Diff Detail

Repository
R241 KIO
Lint
Lint Skipped
Unit
Unit Tests Skipped
wbauer created this revision.Feb 25 2020, 3:22 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptFeb 25 2020, 3:22 PM
wbauer requested review of this revision.Feb 25 2020, 3:22 PM
wbauer edited the test plan for this revision. (Show Details)Feb 25 2020, 3:37 PM
wbauer updated this revision to Diff 76387.Feb 25 2020, 4:17 PM

Use KShell::quoteArg() in case the icon name contains spaces or other special chars.
No idea if that's possible/allowed, but better be safe than sorry I suppose.

Since this is konsole specific, it's fine to use a Qt-specific argument.
But IMHO you could also just remove the %i altogether.
I never understood the idea of "take the icon from the desktop file, not the one the application would use by default".
I mean, with some stretch of the imagination I can see N desktop files launching the same app with different options including the icon (why not...), but not like here where it will just look up the default konsole.desktop anyway.

Wait, you're actually passing the icon of the command being executed now (while the old code would end up using the stuff from konsole.desktop I think)....
But it gets worse, konsole is a unique-application by default, isn't it? So this icon will never show, unless this is the first app that starts konsole -- and then all subsequent apps will end up with that icon of the first command.... This is all nonsense to me....

Option 1: remove %i
Option 2: test all the scenarios above... (icon in app desktop file + konsole not already running, then launching konsole again ; icon in app desktop file + konsole already running with default parameters)

wbauer added a comment.EditedFeb 26 2020, 11:44 AM

Wait, you're actually passing the icon of the command being executed now (while the old code would end up using the stuff from konsole.desktop I think)....

AFAICT passing the icon of the command being executed is what is wanted here.
I.e. the konsole window should use the icon of the application menu entry it is being started from.
And the original code (with %i) actually does the same, I verified that with additional debug output.

For some reason, passing -qwindowicon (or --icon for that matter) doesn't have any visible effect in Plasma, not even on X11 (maybe kwin5 overrides the window icon?).
It does work on other desktops hower, also running several konsole instances with different window icons.

The intention of this patch was/is to not change the existing behavor for whatever it's worth, only fix the problem at hand.

dfaure accepted this revision.Feb 27 2020, 9:20 PM

Hum. OK. Weird.

This revision is now accepted and ready to land.Feb 27 2020, 9:20 PM
This revision was automatically updated to reflect the committed changes.