Review Request 120606: Properly parse URL in KUrlNavigator

[David Faure]

> On Oct. 16, 2014, 2:09 p.m., Aleix Pol Gonzalez wrote:
> > Can we maybe get a unit test for that?
> 
> Jan Grulich wrote:
>     I don't think this is necessary, just take a look how it was implemented before [1].
>     
>     QString pathOrUrl = currentUrl.pathOrUrl(); vs QString path = currentUrl.toString(QUrl::PreferLocalFile);
>     and
>     newUrl.setPath(KUrl(pathOrUrl).path()); vs newUrl.setPath(QUrl(path).path());
>     
>     I think this one is one of those bugs introduced during KUrl ? QUrl port.
>     
>     [1] - http://api.kde.org/4.x-api/kdelibs-apidocs/kfile/html/kurlnavigator_8cpp_source.html

KUrl(pathOrUrl) was correct, QUrl(pathOrUrl) is wrong. A unittest, with a testcase containing a '#' in the directory name, would be very useful (but I would agree that it's not fair to *require* that you, trying to fix a bug in code that is not unit-tested, *have* to write a unittest from scratch. We can suggest it, though ;) ).


- David


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


On Oct. 16, 2014, 1:56 p.m., Jan Grulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120606/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2014, 1:56 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> When using an URL with a scheme, like sftp://foo@bar.com/home/foo/, then KUrlNavigator doesn't properly parse it. At the beginning it tries to count the number of slashes from sftp://foo@bar.com, which is 2, then it tries to contruct buttons using names from particular sections separated by slashes, but when we use only QUrl::path() for URL above, we will always get only "/home/foo/" path and therefore we will have bigger index then the number of sections, which leads to have same URLs for all buttons. We need to parse sections from full URL including first two slashes.
> 
> 
> Diffs
> -----
> 
>   src/filewidgets/kurlnavigator.cpp e96d914 
> 
> Diff: https://git.reviewboard.kde.org/r/120606/diff/
> 
> 
> Testing
> -------
> 
> Tested in dolphin and works fine now.
> 
> 
> File Attachments
> ----------------
> 
> Screenshot of the problem
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/16/eda6fb7e-a75e-4c28-ac87-5d16a39a366d__kurlnavigator.png
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141016/73161d01/attachment.html>