D27654: [kio] Fix running konsole on Wayland

2020-02-28 Thread Wolfgang Bauer
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:07ab04bfe774: Fix running konsole on Wayland (authored by 
wbauer).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27654?vs=76387=76607

REVISION DETAIL
  https://phabricator.kde.org/D27654

AFFECTED FILES
  src/core/desktopexecparser.cpp

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-27 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Hum. OK. Weird.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27654

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-26 Thread Wolfgang Bauer
wbauer added a comment.


  In D27654#617946 , @dfaure wrote:
  
  > 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.
  
  It's true that 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27654

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-25 Thread David Faure
dfaure added a comment.


  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)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27654

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-25 Thread Wolfgang Bauer
wbauer updated this revision to Diff 76387.
wbauer added a comment.


  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.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27654?vs=76375=76387

REVISION DETAIL
  https://phabricator.kde.org/D27654

AFFECTED FILES
  src/core/desktopexecparser.cpp

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-25 Thread Wolfgang Bauer
wbauer edited the test plan for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27654

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D27654: [kio] Fix running konsole on Wayland

2020-02-25 Thread Wolfgang Bauer
wbauer created this revision.
wbauer added a reviewer: dfaure.
wbauer added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
wbauer requested review of this revision.

REVISION 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.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27654

AFFECTED FILES
  src/core/desktopexecparser.cpp

To: wbauer, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns