KDE/kdelibs/kio/kio

[Jeff Mitchell]

David Faure wrote:
> I am not familiar with the decisions taken for KUrl on Windows. Is C:/Documents%20and%20Settings/jeff/
> supposed to be a path or a URL?

A path, except that it's incorrectly escaped when it shouldn't be.

> So if this applies on Windows too, I assume the above is a URL.

No, a URL on windows would be (I don't have everything percent-encoded 
here):
file:///C:/Documents%20and%20Settings/blahblahblah

Your assumption is not correct.

, so I don't see how it could have fixed the above. I guess you mean that
> you had the opposite, C:/Document and Settings/jeff/My%20Documents, rather....

Yeah, sorry.  I haven't looked at it in a long time (three months). 
This is what I mean.

> Actually, in a URL, percent-escaping of space is optional. So the above URL is quite fine.
> It can't look stupid, because it shouldn't be shown to the user as is, you're supposed to show
> prettyUrl() or path() to the user, both of which would use ' ' everywhere anyway.

IIRC, these weren't working correctly because of other issues.  I really 
don't remember the specifics though.

>> The problem was that part of it was being treated as a URL and part of 
>> it as a path.  So what my patch did was it queried the url to see if it 
>> really was a URL ( isURL() ) and if the answer came back yes, it would 
>> percent-encode what it was appending.  Otherwise, it wouldn't, since the 
>> URL wouldn't actually be a URL.
> 
> No that's what my patch does :-)
> Your patch did the opposite, i.e. always appending without percent-encoding :)

Yeah, I didn't look carefully enough.

I think I'm remembering the issue.  I think it had to do with C:/ 
messing up the detection of whether it was a URL or path -- it's a path 
but was getting detected as a URL with protocol C.  I may have done some 
hacking around in kurl.cpp to fix it.

> r793612 | mitchell | 2008-04-04 17:06:05 +0200 (Fri, 04 Apr 2008) | 1 line
> 
> Remove complete_url check, it causes URLs to be completed with the last part percented-encoded but not the rest.
> ------------------------------------------------------------------------
> Index: kurlcompletion.cpp
> ===================================================================
> --- kurlcompletion.cpp  (revision 793611)
> +++ kurlcompletion.cpp  (revision 793612)
> @@ -380,7 +380,6 @@ public:
>         KUrlCompletion::Mode mode; // ExeCompletion, FileCompletion, DirCompletion
>         bool replace_env;
>         bool replace_home;
> -       bool complete_url; // if true completing a URL (i.e. 'prepend' is a URL), otherwise a path
> 
>         KIO::ListJob *list_job; // kio job to list directories
> 
> @@ -628,7 +627,6 @@ QString KUrlCompletion::makeCompletion(c
>         if ( url.kurl()->hasRef() )
>                 toRemove += url.kurl()->ref().length() + 1;
>         d->prepend = text.left( text.length() - toRemove );
> -       d->complete_url = url.isURL();
> 
>         QString aMatch;
> 
> @@ -1075,10 +1073,6 @@ void KUrlCompletionPrivate::addMatches(
>         QStringList::ConstIterator it = matchList.begin();
>         QStringList::ConstIterator end = matchList.end();
> 
> -       if ( complete_url )
> -               for ( ; it != end; ++it )
> -                       q->addItem( prepend + QUrl::toPercentEncoding(*it) );
> -       else
>                 for ( ; it != end; ++it )
>                         q->addItem( prepend + (*it));
>  }
> 
> 
>> So I think your problem, whatever it is, might be in isURL returning 
>> false when it should return true.
> 
> But I don't have a problem (anymore, after my commit). The question is what
> your problem was/is again :)

So, after my patch (which removed that complete_url check and percent 
encoding), I believe that on Windows I saw the correct behavior of 
C:/Documents and Settings vs. file:///C:/Documents&and&Settings.  But it 
may be a moot point now if I eventually managed to fix the detection 
being screwy in the first place.

KDE-Windows guys, can any of you chime in?  Have you tried using 
anything with kurlcompletion (like dolphin) since this revert?  Are 
paths and URLs being appropriately escaped/not escaped?

--Jeff