D7479: klauncher: fix appId matching for flatpak apps
This revision was automatically updated to reflect the committed changes. Closed by commit R303:1ebdf8ea6ac1: klauncher: fix appId matching for flatpak apps (authored by elvisangelaccio). REPOSITORY R303 KInit CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7479?vs=18758=18856 REVISION DETAIL https://phabricator.kde.org/D7479 AFFECTED FILES src/klauncher/klauncher.cpp To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
dfaure accepted this revision. REPOSITORY R303 KInit BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio updated this revision to Diff 18758. elvisangelaccio added a comment. Use `leftRef()` REPOSITORY R303 KInit CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7479?vs=18700=18758 BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 AFFECTED FILES src/klauncher/klauncher.cpp To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Now that I think about it, leftRef() would be even better... REPOSITORY R303 KInit BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
dfaure added inline comments.
INLINE COMMENTS
> klauncher.cpp:326
> {
> #ifdef KLAUNCHER_VERBOSE_OUTPUT
> qCDebug(KLAUNCHER) << pid << "exitStatus=" << exitStatus;
This seems to be mixed with an unrelated change, now.
REPOSITORY
R303 KInit
REVISION DETAIL
https://phabricator.kde.org/D7479
To: elvisangelaccio, dfaure, apol
Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
dfaure added a comment. Hint: `arc diff HEAD~` REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio marked 2 inline comments as done. REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio updated this revision to Diff 18700. elvisangelaccio added a comment. - Fixed David's issues. REPOSITORY R303 KInit CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7479?vs=18600=18700 BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 AFFECTED FILES src/klauncher/klauncher.cpp To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
dfaure added inline comments.
INLINE COMMENTS
> klauncher.cpp:380
> +// Match sandboxed apps (e.g. flatpak), see
> https://phabricator.kde.org/D5775
> +if (newAppId.endsWith(QStringLiteral(".kdbus"))) {
> +return newAppId.startsWith(pendingAppId);
QString::endsWith() has a QLatin1String overload, which would be slightly
better here.
> klauncher.cpp:381
> +if (newAppId.endsWith(QStringLiteral(".kdbus"))) {
> +return newAppId.startsWith(pendingAppId);
> +}
So if I start kdesudoku and kdesu quickly, when kdesudoku is started, the
pending entry for kdesu will think it matches? ;)
It would be technically more correct to do newAppId.left(newAppId.length() - 6)
== pendingAppId, I think.
REPOSITORY
R303 KInit
REVISION DETAIL
https://phabricator.kde.org/D7479
To: elvisangelaccio, dfaure, apol
Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio edited the summary of this revision. REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio updated this revision to Diff 18600. elvisangelaccio added a comment. - Add reference to https://phabricator.kde.org/D5775 REPOSITORY R303 KInit CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7479?vs=18585=18600 BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 AFFECTED FILES src/klauncher/klauncher.cpp To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio added a comment. In https://phabricator.kde.org/D7479#138860, @elvisangelaccio wrote: > I just realized that ".kdbus" is appended by KDBusAddons, not by flatpak. Maybe we could also fix it there... Nope, it's intentional (see https://phabricator.kde.org/D5775). So this fix in klauncher should be ok. Btw kate is also affected, while for some reasons okular is not. REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio added a comment. I just realized that ".kdbus" is appended by KDBusAddons, not by flatpak. Maybe we could also fix it there... REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D7479 To: elvisangelaccio, dfaure, apol Cc: #frameworks
D7479: klauncher: fix appId matching for flatpak apps
elvisangelaccio created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY With flatpak applications we can have the following scenario: appId= "org.kde.ark.kdbus-_1_769" newAppId= "org.kde.ark.kdbus" pendingAppId= "org.kde.ark" This breaks matchesPendingRequest(), resulting in an annoying `"KDEInit could not launch '/usr/bin/flatpak'"` dialog after closing the app. This patch adds an explicit check for this case. TEST PLAN - install ark flatpak - kde-open5 foo.zip - close ark REPOSITORY R303 KInit BRANCH kdbus-appId REVISION DETAIL https://phabricator.kde.org/D7479 AFFECTED FILES src/klauncher/klauncher.cpp To: elvisangelaccio, dfaure, apol Cc: #frameworks
