[KFilePlacesModel] Fix supported scheme check for devices
ClosedPublic

Authored by broulik on Dec 14 2019, 3:13 PM.

Details

Summary

A device doesn't usually have a URL so we need to actutally check the underlying device (e.g. StorageAccess or NetworkShare) for whether it is supported.

Test Plan
  • kdialog --getopenfilename ~ now has my mounted ISOs and external storage now
  • didn't test with CD drives or network shares
  • KFilePlaces tests stil pass

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.Dec 14 2019, 3:13 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 14 2019, 3:13 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Dec 14 2019, 3:13 PM
ngraham accepted this revision.Dec 14 2019, 3:28 PM
ngraham added a subscriber: ngraham.

Oh so nice.

This revision is now accepted and ready to land.Dec 14 2019, 3:28 PM
bruns added a subscriber: bruns.Dec 14 2019, 4:32 PM
bruns added inline comments.
src/filewidgets/kfileplacesmodel.cpp
758

you have dropped the allowedHere check. Probably just if (!allowedHere) continue.

And for reasons unknown to me, tags: is not filtered per app - move the allowedHere check to the very beginning of the while(...) loop.

759

Does KFilePlacesItem have a move constructor? You can do the heap allocation then only if necessary.

bruns requested changes to this revision.Dec 14 2019, 4:32 PM
This revision now requires changes to proceed.Dec 14 2019, 4:32 PM
broulik added inline comments.Dec 14 2019, 5:55 PM
src/filewidgets/kfileplacesmodel.cpp
758

Devices are always allowed. The old check was

if (isSupportedScheme && ((isSupportedUrl && ... && allowedHere) || deviceAvailable))

so deviceAvailable trumps allowedHere

759

It does not.

No response, I think you can land this.

This revision was not accepted when it landed; it landed in state Needs Revision.Fri, Jan 3, 6:59 PM
This revision was automatically updated to reflect the committed changes.
Restricted Application added a project: Frameworks. · View Herald TranscriptFri, Jan 3, 6:59 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript