[Task Tools] Treat applications: and respective absolute path equal in launcherUrlsMatch
ClosedPublic

Authored by broulik on Oct 12 2017, 8:56 AM.

Details

Summary

BUG: 385594

Test Plan

Having a file:///usr/share/applications/org.kde.dolphin.desktop launcher is no longer juggled around as I launch and quit Dolphin. Thanks Michail and Nate for pointing me to the right place (launcherPosition)

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Oct 12 2017, 8:56 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 12 2017, 8:56 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik edited the test plan for this revision. (Show Details)Oct 12 2017, 8:57 AM
broulik updated this revision to Diff 20627.Oct 12 2017, 9:12 AM
  • Use same approach as appDataFromUrl (ie. KDesktopFile::isDesktopFile and then KDesktopFile fileName
davidedmundson added inline comments.
libtaskmanager/tasktools.cpp
616

launcherUrslMatch is in a somewhat hot path that is called a lot.

If we have to do this, fine, but if there's an alternative; such as resolving the launcher to a KService (KService::serviceByMenuId) once in the launcher, that would be IMHO much much better.


Also menuId is built from the filename; (vfolder_menu.cpp:947) (prefix in that line is empty for anything non-supersuper-legacy)

Is there a reason we can't just compare filenames here?

broulik added inline comments.Oct 13 2017, 10:45 AM
libtaskmanager/tasktools.cpp
616

resolving the launcher to a KService (KService::serviceByMenuId) once in the launcher,

?

Is there a reason we can't just compare filenames here?

Don't know, I just looked at what the rest of the code dealing with applications: URLs does.

hein accepted this revision.Oct 13 2017, 5:44 PM
This revision is now accepted and ready to land.Oct 13 2017, 5:44 PM
hein added a comment.Oct 13 2017, 5:46 PM

Let's merge this for now to have a fix for 5.11.1. I'll look into speeding things up next week.

This revision was automatically updated to reflect the committed changes.