KFileWidget treats filenames as complete urls, which causes https://github.com/openSUSE/kmozillahelper/issues/7 ('Save file dialog does not prefill filename when title have "xxx : xxx"') This quick snippet has a default filename filled in without the KDE platform theme but with it, the filename input is empty and this message appears: "this is a test with a colon : hi.file" is not a correct argument for setSelection! "this is a test with a colon : hi.file" is not a correct argument for setSelection! #include <QApplication> #include <QFileDialog> int main(int argc, char *argv[]) { QApplication a(argc, argv); QUrl defaultUrl = QUrl::fromLocalFile("file:///tmp/this is a test with a colon : hi.file"); QFileDialog dialog(nullptr, "Broken", defaultUrl.path()); // Does not work dialog.selectFile(defaultUrl.fileName()); // Does not work either //dialog.selectUrl(defaultUrl); dialog.show(); return a.exec(); }
Workaround is to use dialog.selectFile(QUrl::toPercentEncoding(defaultUrl.fileName())); but that unfortunately breaks with non-KDE dialogs and also after this bug is fixed...
I made a review request with a fix for this in plasma-integration: https://phabricator.kde.org/D4579 However, IMO the API needs to be changed to be less ambiguous.
Git commit e70f8134a2bc4b3647e245c05f469aeed462a246 by Fabian Vogt. Committed on 13/02/2017 at 15:11. Pushed by fvogt into branch 'Plasma/5.8'. Do not treat filename in selection as URL Summary: KFileWidget::setSelection(QString &) accepts either absolute URLs or relative paths. If the filename contains a :, it gets treated as a URL and gets rejected. This forces setSelection to parse it as URL. Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D4579 M +1 -1 src/platformtheme/kdeplatformfiledialoghelper.cpp https://commits.kde.org/plasma-integration/e70f8134a2bc4b3647e245c05f469aeed462a246
Thanks for fixing the bug, it can be closed now, right?
(In reply to David Faure from comment #4) > Thanks for fixing the bug, it can be closed now, right? IMO not quite: > However, IMO the API needs to be changed to be less ambiguous. A QString as argument that accepts both URLs and filenames can't ever work as valid URLs are also valid filesystem paths.
Oh, indeed, I hadn't seen that. I completely agree, the same API with a QString can't be for relative paths and for URLs, "a:b" is both. How about we add a KFileWidget::setSelectedUrl(const QUrl& url)? Then we can get rid of this ugly .toString() (QUrl -> QString -> QUrl).
(In reply to David Faure from comment #6) > Oh, indeed, I hadn't seen that. I completely agree, the same API with a > QString can't be for relative paths and for URLs, "a:b" is both. > > How about we add a KFileWidget::setSelectedUrl(const QUrl& url)? > Then we can get rid of this ugly .toString() (QUrl -> QString -> QUrl). Sounds good to me!
https://phabricator.kde.org/D4937
Git commit f49f34727ee68e8b86debc1e7bb24f113505f890 by David Faure. Committed on 08/03/2017 at 12:11. Pushed by dfaure into branch 'master'. Add KFileWidget::setSelectedUrl() Summary: It turns out that setSelection() cannot take both relative paths (filenames) and absolute URLs as input. "a:b" can be both, and URLs for files called "a#b" were handled wrongly. Reviewers: fvogt Reviewed By: fvogt Subscribers: fvogt, #frameworks Tags: #frameworks Differential Revision: https://phabricator.kde.org/D4937 M +85 -6 autotests/kfilewidgettest.cpp M +9 -3 src/filewidgets/kfilewidget.cpp M +23 -3 src/filewidgets/kfilewidget.h https://commits.kde.org/kio/f49f34727ee68e8b86debc1e7bb24f113505f890
And now we can do the other bit, please test https://phabricator.kde.org/D6350.
Landed in b785b9c.