Review Request: ftp server URL throws "invalid protocol" - Bug: 209031

[Mark]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/5148/
-----------------------------------------------------------

(Updated 2010-08-26 14:26:16.882782)


Review request for Dolphin, kdelibs and David Faure.


Changes
-------

added dfaure to people list
patched against kdelibs 4.5 tag instead of branch since that gives me more certainty that is something is broken.. i did it ^_^

Diff
-----
modified with the suggestions from Peter
Deleted the "~" check (not added by me, but debugged it and it's already done in "QString urlStr = KUrlCompletion::replacedPath(url.url(), true, true);" where the first true seems to cause it and it's actually done in: "KUrlCompletionPrivate::MyURL::init")

Tested the patch and it works fine.

Note: after even deeper digging i found the file: "kurlcompletion.cpp/h" which has the function: "KUrlCompletionPrivate::MyURL::init" : http://websvn.kde.org/tags/KDE/4.5.0/kdelibs/kio/kio/kurlcompletion.cpp?view=markup where it already filters on various things (man:, info:, $ and ~)... It might be best to add this check in there since it already seems to be the place to check for these things...


Summary
-------

Read the bug report for the.. bug..

As for the fix. I had a hard time finding the right place to implement this since all was using const KUrl thus not editable... When searching deeper i ended up in making the fix in either KUrlNavigator or in KUrl itself. I decided to go for KUrlNavigator for no particular reason... I could just as well have tried it in KUrl. I hope this KUrlNavigator was the right place (either way, please do explain where is the right place and why the other one isn't the right place). 


This addresses bug 209031.
    https://bugs.kde.org/show_bug.cgi?id=209031


Diffs (updated)
-----

  /tags/KDE/4.5.0/kdelibs/kfile/kurlnavigator.cpp 1168303 

Diff: http://reviewboard.kde.org/r/5148/diff


Testing
-------

I did one simple tests on it:
typing "ftp.nluug.nl" in the url bar and pressing enter. It changed the url to "ftp://ftp.nluug.nl" and the ftp opened just perfectly. I browsed a bit on it to see if it stays working fins and it seems to do just that.


Thanks,

Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100826/5ab5cf99/attachment.htm>