Review Request 114693: fix KFileWidget url selection

David Faure faure at kde.org
Mon Dec 30 21:54:26 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114693/#review46461
-----------------------------------------------------------


Ah I see, the issue is when calling setLocationText() with a relative URL.

The KDE4 code (inside setLocationText) would say

if (url.hasPath()) {
    if (!url.directory().isEmpty()) {
        q->setUrl(...);
    } else {
        q->setUrl(url.path(), false);
    }
}

When called with a relative url, url.path() is "passwd", we end up in 
KDirOperator::setUrl("passwd"), which errors out in the KIO::NetAccess::stat check.

Much work for nothing indeed, we should just skip the whole setUrl when called with a relative URL.

This hasPath() was wrong, it should have been a "is not relative" check.

Back to KF5: does this fix it?

-        if (!url.path().isEmpty()) {
+        if (!url.isRelative()) {

I'm curious as to why the setUrl("passwd") behaved differently from kde4 though?



- David Faure


On Dec. 30, 2013, 9 p.m., Michal Humpula wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114693/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 9 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> If I understand correctly documentation of KFileWidget, it should be perfectly ok to do something like this:
> 
> KEncodingFileDialog::getOpenUrlsAndEncoding(QString(), 
> QUrl("file:///etc/passwd"));
> 
> But that doesn't display the thing I'm expecting. Tracing it down I came up with the fix. I'm not claiming that it's the correct one, but at least in my situation the KFileWidget behaves as expected in all tested situations. 
> 
> Surprisingly the
> 
> void KFileWidgetPrivate::setLocationText(const QList<QUrl> &urlList)
> 
> doesn't call any setUrl, which hints that it could actually be correct.
> 
> 
> Diffs
> -----
> 
>   src/filewidgets/kfilewidget.cpp 11597b3 
> 
> Diff: https://git.reviewboard.kde.org/r/114693/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michal Humpula
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131230/1c1bea2e/attachment.html>


More information about the Kde-frameworks-devel mailing list