D24043: [TagDialog] Really enable "Open in filemanager" button for local files
Wolfgang Bauer
noreply at phabricator.kde.org
Wed Sep 18 09:17:33 BST 2019
wbauer created this revision.
wbauer added a reviewer: Amarok.
wbauer added a project: Amarok.
Herald removed a project: Amarok.
Herald added a subscriber: amarok-devel.
wbauer requested review of this revision.
REVISION SUMMARY
`QUrl::QUrl()` expects a full Url including the scheme (unlike `KUrl::KUrl()` that supports local paths too).
But for local files, `urlString` contains the local path without a scheme ("file://").
The problem here is that `QUrl` cannot detect that the Url is actually pointing to a local file if the scheme is missing (according to the Qt docs, a URL is a local file path if the scheme is "file"), so `QUrl::isLocalFile()` was always `false` and the "Open in filemanager" button never got enabled.
So use `QUrl::fromUserInput()` instead to construct the `QUrl`, which also supports local paths.
Also, to avoid showing the "file://" scheme in the "Location" text field now, pass `QUrl::PreferLocalFile` to `QUrl::toDisplayString()` as suggested in the KUrl::pathOrUrl() docs <https://api.kde.org/frameworks/kdelibs4support/html/classKUrl.html#a4f0c69c2e82bd69a9dfc9c84439b29ea>.
TEST PLAN
Opened the tag dialog ("Edit track details") for a file in the local collection.
The button is enabled now, clicking it successfully opens the containing folder in dolphin.
The "Location" text field still shows "/path/to/folder/file" (without "file://").
Added a remote podcast to the playlist, and opened the tag dialog.
The button stays disabled, the text field still correctly shows the Url including the scheme ("https://...") in my case).
REPOSITORY
R181 Amarok
REVISION DETAIL
https://phabricator.kde.org/D24043
AFFECTED FILES
src/dialogs/TagDialog.cpp
To: wbauer, #amarok
Cc: amarok-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20190918/1297a018/attachment.html>
More information about the Amarok-devel
mailing list