----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126831/#review91468 -----------------------------------------------------------
src/filewidgets/kfilewidget.cpp (line 1462) <https://git.reviewboard.kde.org/r/126831/#comment62519> I think we should, yes, or even in the caller of setDirectory, if it's calling it with a non-directory URL. From the name it's pretty clear that we don't expect that to be called with a file. - David Faure On Jan. 20, 2016, 9 p.m., Alex Richardson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126831/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2016, 9 p.m.) > > > Review request for 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
