-----------------------------------------------------------
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 (updated)
-----

  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

Reply via email to