Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[David Faure]

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



src/filewidgets/fix-url-navigator.patch (line 1)
<https://git.reviewboard.kde.org/r/126295/#comment61062>

    I don't think you wanted to added this patch to the git repo :-)



src/filewidgets/kurlnavigator.cpp (line 361)
<https://git.reviewboard.kde.org/r/126295/#comment61063>

    This can be a local path, you can't give it to the QUrl(QString) constructor (it would break e.g. when the filename contains '#')
    
    It seems retrievePlacePath() can return either a local path or a URL. I would change that to always return a URL, that's much easier to handle programmatically. Use toDisplayString(PreferLocalFile) only at the time of displaying this to the user.
    retrievePlacePath() doing idx+=3 and url.left(idx) is indeed very very wrong, in terms of path escaping.


- David Faure


On Dec. 9, 2015, 9:42 p.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126295/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2015, 9:42 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> 9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. The argument index passed to buttonUrl is based number of '/'in full url. Because of that, url like "sftp://a@b.com/a/b" is now shown as "sftp:a at b.com" / "b" / "b" in dolphin url bar.
> 
> This patch changes all relevant code that calls buttonUrl() to use url.path() instead of url.toDisplayString() to count the number of "/".
> 
> 
> Diffs
> -----
> 
>   src/filewidgets/fix-url-navigator.patch PRE-CREATION 
>   src/filewidgets/kurlnavigator.cpp 848e98b 
> 
> Diff: https://git.reviewboard.kde.org/r/126295/diff/
> 
> 
> Testing
> -------
> 
> Remote url -> ok
> Local url -> ok
> Remote url in places and try browse some sub folder -> ok
> Local url in places and try browse some sub folder -> ok
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151210/d631edfd/attachment-0001.html>