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

[David Faure]

> On 2010-08-26 16:34:56, Rolf Eike Beer wrote:
> > Why is that startsWith("ftp.") and not just startsWith("ftp")? I'm always annoyed Konqueror gets ftp5.gwdg.de wrong ;)
> 
> Mark wrote:
>     simple. If i do startsWith("ftp") then anything that starts with "ftp" will get the ftp:// prefix..
>     Besides, ftp. and ftp://ftp. is common. ftp5. is far from common if you ask me ^_^ And changing this would mean that anything before the point that contans "ftp" should get ftp:// ... i'm not sure if that's a good idea although not hard to make.
>     
>     everything before the first point then .contains("ftp") and voila.. ^_^
>     
>     But, someone else.. Is this a required change?
> 
> Dawit Alemayehu wrote:
>     @Mark:
>     The actual fix should be to replace the "KUrlCompletion::replacedPath" call with "KURIFilter::self()->filterUri" in that function as follows:
>     
>     // Use short url and web shortcut filters of the uri input filtering class...
>     if (!KURIFilter::self()->filterUri(url, QStringList() << "kshorturifilter" << "kurisearchfilter"))
>         return;
>     
>     @Rolf:
>     Since Konqueror already uses KURIFilter to filter user input, you can simply use the undocumented feature of the kshorturifilter plugin to solve your annoyance...
>     
>     #1.) Create a $KDEHOME/share/config/kshorturifilterrc or copy the global one that comes with KDE by default.
>     #2.) Add the following to the file:
>     [Pattern]
>     myftpsite=ftp5.gwdg.de
>     
>     [Protocol]
>     myftpsite=ftp://
>     
>     Restart konqueror... Now whenever you type that url, ftp:// will be used instead of the default http://. You can add as many pattern/protocol associations as well as use regular expressions for the pattern. KDE comes with a pre installed global kshorturifilterrc file that contains the following:
>     
>     [Pattern]
>     kdemailto=^(\\w+)(?:[.]\\w+)?@(\\w+)(?:[.]\\w+)+$
>     kdeftp=^ftp(?:\\.\\w+)?\\.\\D{2,}(?:[0-9]{1,5})?
>     
>     [Protocol]
>     kdemailto=mailto:
>     kdeftp=ftp://
> 
> Mark wrote:
>     @Dawit Alemayehu
>     
>     those filters are interesting indeed, but i don't get a few (kinda vital) things with it.
>     1. Where and how do i make the filters?
>     2. How is that filter stuff knowing all the filters like it says in the docs?
>     3. where is a list of existing filters?
>     
>     The docs say this filter stuff is simple but i really miss an example of how it's all tight together...
>     Any help with that would be welcome.
> 
> Dawit Alemayehu wrote:
>     #1. It is all automagically done for you. That is you do not need to write any filters yourself. Several filter plugins are already available in the location mentioned below (see #3). The issue you are trying to resolve is specifically handled by the presence of the "kdeftp=" pattern shown above. All you need to do is use the exact code I provided above (sans the wrong class name) or if you want a more detailed response from the plugin, you can use KUriFilterData as follows:
>     
>     KUriFilterData data;
>     data.setData(url);
>     data.setCheckExecutables(false);  // Do not filter an executable...
>     if (KUriFilter::self()->filterUri(data, QStringList() << "kshorturifilter" << "kurisearchfilter"))
>         url = data.uri();
>     
>     data.uriType() would tell you what the filtered resource type is, etc...
>     
>     #2. KURIFilter is a plugin based system. In other words, the actual filtering is done by plugins that are dynamically loaded when you get an instance of the class through self(). If no filter plugins are found, i.e. not installed, then the filtering process will fail. You can find out how many filters are available by calling calling "KURIFilter::self()->pluginNames()" function.
>     
>     #3. Currently all the filter plugins are located under kdebase/runtime/kuri-filterplugins and unfortunately that might be an issue against using KURIFilter in this instance, but then again KUriFilter is used by kparts, khtml and kdewebkit all in kdelibs ; so this should not be a problem to use it in this case either...

Yes #3 is a non-issue: kdebase/runtime *is* the set of runtime dependencies for kdelibs, by definition.

The question is whether we really want all the effects of kshorturifilter+kurisearchfilter in kurlnavigator: e.g. should gg:foo to go to a google search, in kurlnavigator. Initially my reaction was: no, since in dolphin this makes no sense. But thinking about it again, being able to use kurlnavigator in konqueror would be quite nice actually.

We have no way of doing "directory only" uri filtering. E.g. one could argue that typing an IP address in dolphin should open sftp://ip rather than http://ip (like kshorturifilter does). Which would require some sort of settings being passed to kshorturifilter.

Anyway, this is all ice on the cake. I'm not against a simple use of kurifilter for now, the above nitpicking can be done separately.


- David


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


On 2010-08-26 14:26:16, Mark wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5148/
> -----------------------------------------------------------
> 
> (Updated 2010-08-26 14:26:16)
> 
> 
> Review request for Dolphin, kdelibs and David Faure.
> 
> 
> 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
> -----
> 
>   /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/bfe840a2/attachment.htm>