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