Differential D25991

[KFilePlacesModel] Fix supported scheme check for devices
ClosedPublic
Actions

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

Details

Reviewers
dfaure
ngraham
bruns
Group Reviewers
Frameworks
Commits
R241:3a6e3f927111: [KFilePlacesModel] Fix supported scheme check for devices
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
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
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.

ngraham added a comment.Jan 3 2020, 6:48 PM

No response, I think you can land this.

This revision was not accepted when it landed; it landed in state Needs Revision.Jan 3 2020, 6:59 PM
Closed by commit R241:3a6e3f927111: [KFilePlacesModel] Fix supported scheme check for devices (authored by broulik). · Explain Why
This revision was automatically updated to reflect the committed changes.
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 3 2020, 6:59 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

Revision Contents
Changeset List

Diff 71522

View Options

src/filewidgets/kfileplacesitem.cpp

Loading...
View Options

src/filewidgets/kfileplacesitem_p.h

Loading...
View Options

src/filewidgets/kfileplacesmodel.cpp

Loading...