Review Request 126831: WIP: Fix QFileDialog::openUrl() for remote files
Alex Richardson
arichardson.kde at gmail.com
Wed Jan 20 20:58:51 UTC 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126831/
-----------------------------------------------------------
Review request for Kate and KDE Frameworks.
Bugs: 345253
https://bugs.kde.org/show_bug.cgi?id=345253
Repository: kio
Description
-------
WIP: Fix QFileDialog::openUrl() for remote files
QFileDialog doesn't make an attempt to determine whether remote URLs are files or directories so it [just passes the file URL to the platform file dialog](http://code.woboq.org/qt5/qtbase/src/widgets/dialogs/qfiledialog.cpp.html#_ZL17_qt_get_directoryRK4QUrl) which currently causes an error popup and a nonexistent directory to be selected
I will clean up the code and remove qDebug() if this is the correct approach to fixing https://bugs.kde.org/show_bug.cgi?id=345253
I prefer fixing this here instead of in KDEPlatformFileDialog as this means that other calls KFileWidget::setUrl() are also fixed.
Should be a better solution than https://git.reviewboard.kde.org/r/126821/
Problem is there are a lot of KIO::stat() calls going on which can be quite slow for remote files. Any suggestions how to fix them?
Diffs
-----
src/filewidgets/kfilewidget.cpp 868928b3f6511c7890e4cf4ba09edc68048649e7
Diff: https://git.reviewboard.kde.org/r/126831/diff/
Testing
-------
kwrite sftp://user@host/home/user/foo.txt, press open button, directory is shown and foo.txt is selected.
However, there are quite a lot of stats going on for a single QFileDialog::getOpenUrl(), probably we should optimize this.
A total of 8 KIO::stat() calls are made when opening a file, I would expect only 2: original URL to find out if it is a directory or a file
and then the selected file to check if it can be opened.
Debug output:
```
startDir QUrl("")
for QUrl("") -> QUrl("file:///auto/homes/alr48") recentDirClass "" fileName ""
startDir after getStartUrl() QUrl("file:///auto/homes/alr48") file name ""
KIO::stat() QUrl("file:///auto/homes/alr48")
stat of QUrl("file:///auto/homes/alr48") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/bar.txt")
```
First StatJob stats QUrl("$HOME") and comes from the constructor with QUrl(), we can remove that by skipping it if url is empty.
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::KFileWidget kfilewidget.cpp 643 0x7fce663f1e05
3 KDEPlatformFileDialog::KDEPlatformFileDialog kdeplatformfiledialoghelper.cpp 96 0x7fce666b0678
4 KDEPlatformFileDialogHelper::KDEPlatformFileDialogHelper kdeplatformfiledialoghelper.cpp 201 0x7fce666b0dbd
5 KdePlatformTheme::createPlatformDialogHelper kdeplatformtheme.cpp 273 0x7fce666a63bc
6 QDialogPrivate::platformHelper qdialog.cpp 94 0x7fce759eaecc
7 QFileDialogPrivate::platformFileDialogHelper qfiledialog_p.h 112 0x7fce75a03f98
8 QFileDialogPrivate::init qfiledialog.cpp 2800 0x7fce759fa737
9 QFileDialog::QFileDialog qfiledialog.cpp 373 0x7fce759f169d
10 QFileDialog::getOpenFileUrls qfiledialog.cpp 2271 0x7fce759f7da3
11 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
12 KWrite::qt_static_metacall moc_kwrite.cpp 131 0x41ea10
13 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
14 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
15 QAction::triggered moc_qaction.cpp 368 0x7fce7574ae7a
16 QAction::activate qaction.cpp 1163 0x7fce7574a28a
17 QAction::trigger qaction.h 177 0x7fce7574b821
18 QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
19 QAbstractButtonPrivate::click qabstractbutton.cpp 515 0x7fce758aab1c
20 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131 0x7fce758ac08c
... <More>
```
Second one for QUrl("sftp://user@host/home/user/foo.txt") has this backtrace:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
4 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
5 QFileDialogPrivate::setDirectory_sys qfiledialog_p.h 368 0x7fce75a043a1
6 QFileDialog::setDirectoryUrl qfiledialog.cpp 976 0x7fce759f3cbb
7 QFileDialogPrivate::init qfiledialog.cpp 2806 0x7fce759fa7cd
8 QFileDialog::QFileDialog qfiledialog.cpp 373 0x7fce759f169d
9 QFileDialog::getOpenFileUrls qfiledialog.cpp 2271 0x7fce759f7da3
10 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
11 KWrite::qt_static_metacall moc_kwrite.cpp 131 0x41ea10
12 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
13 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
14 QAction::triggered moc_qaction.cpp 368 0x7fce7574ae7a
15 QAction::activate qaction.cpp 1163 0x7fce7574a28a
16 QAction::trigger qaction.h 177 0x7fce7574b821
17 QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
18 QAbstractButtonPrivate::click qabstractbutton.cpp 515 0x7fce758aab1c
19 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131 0x7fce758ac08c
20 QToolButton::mouseReleaseEvent qtoolbutton.cpp 609 0x7fce759a5839
... <More>
```
next one on QUrl("sftp://user@host/home/user/") (caused by an url change here, possibly block signals on d->ops during setUrl()?):
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 1545 0x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176 0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
6 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299 0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080 0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515 0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175 0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
12 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
13 KDirOperator::urlEntered moc_kdiroperator.cpp 586 0x7fce663de2b3
14 KDirOperator::setUrl kdiroperator.cpp 1039 0x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 1491 0x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 QFileDialogPrivate::setDirectory_sys qfiledialog_p.h 368 0x7fce75a043a1
19 QFileDialog::setDirectoryUrl qfiledialog.cpp 976 0x7fce759f3cbb
20 QFileDialogPrivate::init qfiledialog.cpp 2806 0x7fce759fa7cd
... <More>
```
and again QUrl("sftp://user@host/home/user/foo.txt") (why is QFileDialogPrivate::restoreFromSettings() setting an URL if one was explicitly requested):
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
4 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
5 QFileDialogPrivate::setDirectory_sys qfiledialog_p.h 368 0x7fce75a043a1
6 QFileDialog::setDirectoryUrl qfiledialog.cpp 976 0x7fce759f3cbb
7 QFileDialogPrivate::restoreFromSettings qfiledialog.cpp 2716 0x7fce759f9c71
8 QFileDialogPrivate::init qfiledialog.cpp 2812 0x7fce759fa817
9 QFileDialog::QFileDialog qfiledialog.cpp 373 0x7fce759f169d
10 QFileDialog::getOpenFileUrls qfiledialog.cpp 2271 0x7fce759f7da3
11 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
12 KWrite::qt_static_metacall moc_kwrite.cpp 131 0x41ea10
13 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
14 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
15 QAction::triggered moc_qaction.cpp 368 0x7fce7574ae7a
16 QAction::activate qaction.cpp 1163 0x7fce7574a28a
17 QAction::trigger qaction.h 177 0x7fce7574b821
18 QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
19 QAbstractButtonPrivate::click qabstractbutton.cpp 515 0x7fce758aab1c
20 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131 0x7fce758ac08c
... <More>
```
which seems to cause another call to KFileWidget::setUrl() (block signals?). Stats QUrl("sftp://user@host/home/user/") as before:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 1545 0x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176 0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
6 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299 0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080 0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515 0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175 0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
12 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
13 KDirOperator::urlEntered moc_kdiroperator.cpp 586 0x7fce663de2b3
14 KDirOperator::setUrl kdiroperator.cpp 1039 0x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 1491 0x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 QFileDialogPrivate::setDirectory_sys qfiledialog_p.h 368 0x7fce75a043a1
19 QFileDialog::setDirectoryUrl qfiledialog.cpp 976 0x7fce759f3cbb
20 QFileDialogPrivate::restoreFromSettings qfiledialog.cpp 2716 0x7fce759f9c71
21 QFileDialogPrivate::init qfiledialog.cpp 2812 0x7fce759fa817
22 QFileDialog::QFileDialog qfiledialog.cpp 373 0x7fce759f169d
23 QFileDialog::getOpenFileUrls qfiledialog.cpp 2271 0x7fce759f7da3
24 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
25 KWrite::qt_static_metacall moc_kwrite.cpp 131 0x41ea10
... <More>
```
And now KDEPlatformFileDialogHelper::show() sets the URL to QUrl("sftp://user@host/home/user/foo.txt") again
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
4 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
5 KDEPlatformFileDialogHelper::initializeDialog kdeplatformfiledialoghelper.cpp 238 0x7fce666b147f
6 KDEPlatformFileDialogHelper::show kdeplatformfiledialoghelper.cpp 310 0x7fce666b1a36
7 QDialogPrivate::setNativeDialogVisible qdialog.cpp 129 0x7fce759eb10c
8 QFileDialog::setVisible qfiledialog.cpp 842 0x7fce759f3571
9 QWidget::show qwidget.cpp 7728 0x7fce757a8921
10 QDialog::exec qdialog.cpp 533 0x7fce759eb8c7
11 QFileDialog::getOpenFileUrls qfiledialog.cpp 2275 0x7fce759f7e08
12 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
13 KWrite::qt_static_metacall moc_kwrite.cpp 131 0x41ea10
14 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
15 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
16 QAction::triggered moc_qaction.cpp 368 0x7fce7574ae7a
17 QAction::activate qaction.cpp 1163 0x7fce7574a28a
18 QAction::trigger qaction.h 177 0x7fce7574b821
19 QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
20 QAbstractButtonPrivate::click qabstractbutton.cpp 515 0x7fce758aab1c
... <More>
```
causing another call (possibly fixable by blocking signals). Stats QUrl("sftp://user@host/home/user/") again:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 1476 0x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 1545 0x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176 0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
6 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299 0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080 0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515 0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175 0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
12 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
13 KDirOperator::urlEntered moc_kdiroperator.cpp 586 0x7fce663de2b3
14 KDirOperator::setUrl kdiroperator.cpp 1039 0x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 1491 0x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp 189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 KDEPlatformFileDialogHelper::initializeDialog kdeplatformfiledialoghelper.cpp 238 0x7fce666b147f
19 KDEPlatformFileDialogHelper::show kdeplatformfiledialoghelper.cpp 310 0x7fce666b1a36
20 QDialogPrivate::setNativeDialogVisible qdialog.cpp 129 0x7fce759eb10c
21 QFileDialog::setVisible qfiledialog.cpp 842 0x7fce759f3571
22 QWidget::show qwidget.cpp 7728 0x7fce757a8921
23 QDialog::exec qdialog.cpp 533 0x7fce759eb8c7
24 QFileDialog::getOpenFileUrls qfiledialog.cpp 2275 0x7fce759f7e08
25 KWrite::slotOpen kwrite.cpp 250 0x41a0c1
... <More>
```
Finally, when selecting a new file (but this one is required as we need to check whether the new file can be opened)
Stats QUrl("sftp://user@host/home/user/bar.txt")
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::slotOk kfilewidget.cpp 993 0x7fce663f4005
3 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 171 0x7fce663fe357
4 QMetaObject::activate qobject.cpp 3730 0x7fce7490fc54
5 QMetaObject::activate qobject.cpp 3595 0x7fce7490f442
6 QAbstractButton::clicked moc_qabstractbutton.cpp 306 0x7fce75bec92a
7 QAbstractButtonPrivate::emitClicked qabstractbutton.cpp 533 0x7fce758aac1f
8 QAbstractButtonPrivate::click qabstractbutton.cpp 526 0x7fce758aabae
9 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131 0x7fce758ac08c
10 QWidget::event qwidget.cpp 8738 0x7fce757ab018
11 QAbstractButton::event qabstractbutton.cpp 1088 0x7fce758abeca
12 QPushButton::event qpushbutton.cpp 673 0x7fce7596c071
13 QApplicationPrivate::notify_helper qapplication.cpp 3712 0x7fce7575b278
14 QApplication::notify qapplication.cpp 3270 0x7fce75758fdf
15 QCoreApplication::notifyInternal2 qcoreapplication.cpp 1013 0x7fce748cf642
16 QCoreApplication::sendSpontaneousEvent qcoreapplication.h 230 0x7fce7575e226
17 QApplicationPrivate::sendMouseEvent qapplication.cpp 2765 0x7fce75757997
18 QWidgetWindow::handleMouseEvent qwidgetwindow.cpp 554 0x7fce757d7a99
19 QWidgetWindow::event qwidgetwindow.cpp 210 0x7fce757d671f
20 QApplicationPrivate::notify_helper qapplication.cpp 3712 0x7fce7575b278
... <More>
```
Thanks,
Alex Richardson
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160120/d8b1ff39/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list