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


FTP requires a file manager, not a web browser. You get a much better 
experience with a FTP url in e.g. dolphin than in firefox, IMHO. Using web 
browsers for FTP urls is a historic relic from before proper network 
transparency in file managers.


konqueror/client/kfmclient.cpp
<http://git.reviewboard.kde.org/r/103524/#comment7637>

    The naming is confusing. From the code, this method really checks if 
konqueror is the preferred service, right?
    
    In fact this sounds very much like 
KonqMainWindow::isMimeTypeAssociatedWithSelf(mimetype). Not that you can call 
that method, but you could compare the implementations, and call this
    isMimeTypeAssociatedWithKonqueror here, or better, isKonquerorPreferred.



konqueror/client/kfmclient.cpp
<http://git.reviewboard.kde.org/r/103524/#comment7638>

    The check for == 100 seems too strict to me, you get 80 in many cases, 
iirc. In fact I'm thinking of removing the accuracy stuff in qt5, because 
there's no good way of doing something with that number...



konqueror/client/kfmclient.cpp
<http://git.reviewboard.kde.org/r/103524/#comment7639>

    The code later on uses "new KRun" for the case of a user-defined browser, 
given that KRun can handle that. Why not factorize this and call that code for 
local files too? It would make the code shorter and easier to maintain.


- David Faure


On Dec. 24, 2011, 4:14 a.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103524/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2011, 4:14 a.m.)
> 
> 
> Review request for KDE Base Apps and David Faure.
> 
> 
> Description
> -------
> 
> The attached patch changes kfmclient so that it
> 
> * honors the user configured browser setting when the specified URL is a 
> local page that is to be handled by a browser.
> * honors the user configured browser setting when the user specified url is 
> ftp.
> 
> 
> This addresses bug 182591.
>     http://bugs.kde.org/show_bug.cgi?id=182591
> 
> 
> Diffs
> -----
> 
>   konqueror/client/kfmclient.cpp 339ba31 
> 
> Diff: http://git.reviewboard.kde.org/r/103524/diff/diff
> 
> 
> Testing
> -------
> 
> Change the browser in the default application list to "firefox" and make sure 
> all of the following commands open the URL in firefox:
> 
> * kfmclient openURL http://www.kde.org 
> * kfmclient openURL /usr/share/doc/qt/html/index.html
> * kfmclient openURL ftp://ftp.kde.org
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

Reply via email to