[KUrlNavigatorPlacesSelector] Use KFilePlacesModel::convertedUrl
ClosedPublic

Authored by broulik on Feb 19 2018, 11:44 AM.

Details

Summary

Fixes opening recent and search URLs from it.

Test Plan

Disabled Places sidebar in Dolphin, clicked the url navigator places selector and successfully opened the "Search for Images" place. Before Dolphin would complain that "search" isn't a known protocol

Diff Detail

Repository
R241 KIO
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.Feb 19 2018, 11:44 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 19 2018, 11:44 AM
broulik requested review of this revision.Feb 19 2018, 11:44 AM
rkflx added a subscriber: rkflx.Feb 19 2018, 1:07 PM

This fix looks pretty much just like a patch I have lying around somewhere. Still I wonder whether we should monkey-patch all present and future users of KFilePlacesModel instead of fixing a single location? Last time I looked at it this was Baloo (e.g. TimelineProtocol::listDir, parseTimelineUrl etc., but I might be wrong on the connection to the search:/ case).

When fixing the fallout from D8332 I discovered what you are changing here will propagate to every app using KUrlNavigator, which is quite common. (Meanwhile I gave up on adding fixes like 50e6fa3ffc49 everywhere, because I realized this has to be solved at Baloo level.)

rkflx accepted this revision.Mar 1 2018, 10:41 PM

Tested this in several apps, works as it should. Code LGTM.

I'd say we should get this patch in right now, we can always figure out a better solution later. After all, KFilePlacesModel::convertedUrl was introduced for a reason.

This revision is now accepted and ready to land.Mar 1 2018, 10:41 PM
This revision was automatically updated to reflect the committed changes.