> On jun 14, 2014, 8:54 a.m., David Faure wrote:
> > What if the file isn't local?
> >
> > Sounds to me like the bug is elsewhere.
> >
> > Of course for local files, showing a local path looks better than a
> > file:/// URL, so this could be improved, but in a way that doesn't break
> > remote files.
> >
> > The use of KUrl::relativeUrl and putting that into a url looks quite
> > suspicious to me, actually.
> > KUrl didn't work well with relative urls.
> >
> > Maybe locationEditCurrentTextList should be changed into a qstringlist
> > rather than a url list...
> >
> > It would also be interesting to check if this works in the KIO framework
> > (Qt5), where I ported it to QUrl...
>
> Mark Gaiser wrote:
> Just tried it out on KF5. The issue as described on 335922 is partly
> present.
>
> Input files:
> f#1.png
> f%1.png
> f#2.png
> f%2.png
>
> Command used:
> kdialog --multiple --getopenfilename ~ "*.png|PNG images"
>
> kdialog output when selecting those four files:
> /home/kde-devel/335922/f#1.png /home/kde-devel/335922/f#2.png
> /home/kde-devel/335922/f%251.png /home/kde-devel/335922/f%252.png
>
> The files with a "#" in them seem to be parsed just fine in KF5. Those
> with a percent sign still get replaced by "%25".
>
>
> Mark Gaiser wrote:
> Note: Something is fuzzy in that slotOk. When i add one debug line to see
> the output of locationEditCurrentTextList i get two results (as if slotOk is
> called twice). The output i get (in a readable way):
> -- First "Orig QUrl list:"
> - QUrl("file:///home/kde-devel/335922/f%231.png")
> - QUrl( "file:///home/kde-devel/335922/f%232.png" )
> - QUrl( "file:///home/kde-devel/335922/f%251.png" )
> - QUrl( "file:///home/kde-devel/335922/f%252.png" )
>
> -- Second "Orig QUrl list:"
> - QUrl("file:///home/kde-devel/335922/f%231.png")
> - QUrl( "file:///home/kde-devel/335922/f%232.png" )
> - QUrl( "file:///home/kde-devel/335922/f%25251.png" ) <-- What happened
> here?
> - QUrl( "file:///home/kde-devel/335922/f%25252.png" ) <-- What happened
> here?
>
> Mark Gaiser wrote:
> "when i add one debug line" : http://p.sc2.nl/pn8jjrqmx :) Sorry for the
> extra noise.
>
> Mark Gaiser wrote:
> More noise :)
> I played with the code a bit and have it working. My changes are only in
> slotOk so that means the actual issue is in there as well. My "patch" smells
> somewhat hackish and i think David will not like this patch at all, hehe.
>
> Here it is: http://p.sc2.nl/ppfe83z6a
>
> The important change is:
> - I got rid of the slotOk recursion in this code path (it's still in
> another else if).
> - Because i don't enter the function for a second time, i had to make
> sure that locationEditCurrentTextList wasn't modified since that list would
> be needed later on. To do that i removed the "now recalculate all paths for
> them being relative in base of the top most url" loop and do that in the
> QStringList construction which was just a few lines down. This seems to work
> just fine and looks a little cleaner as well imho.
>
> I don't know exactly how this code is supposed to work so i hope one of
> the authors of it (quite a few!) could have a look at what's really going
> wrong here.
>
> David Faure wrote:
> By now the paste expired. Please use permanent URLs or reviewboard.
>
> Mark Gaiser wrote:
> I had no clue that this rr was still alive. I'll see if i can figure out
> what i pasted (half a year ago). It might take a few days..
Damn sorry! I don't have the patch anymore. Not in my git nor in the website
archive. The thing i vaguely remember is slotOk being called from within slotOk
(small recursion). That then caused the parsed url's to be re-parsed which in
turn ends up with the wrong results. Or that's what i recall along with the
stuff i wrote above.
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118604/#review60070
-----------------------------------------------------------
On jun 7, 2014, 4:43 p.m., Victor Dodon wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118604/
> -----------------------------------------------------------
>
> (Updated jun 7, 2014, 4:43 p.m.)
>
>
> Review request for kdelibs, Albert Vaca Cintora and Aaron J. Seigo.
>
>
> Bugs: 335922
> http://bugs.kde.org/show_bug.cgi?id=335922
>
>
> Repository: kdelibs
>
>
> Description
> -------
>
> Only when selecting multiple files in KFileDialog, if a file contains in the
> name the characters '#' or '%', it is (wrongly) escaped.
>
> The only modification needed is in kfilewidget.cpp, in slotOk, instead of
> appending to the stringList the pretty url, append the url.toLocalFile, to
> prevent the double escaping.
>
>
> Diffs
> -----
>
> kfile/kfilewidget.cpp fc9b169
>
> Diff: https://git.reviewboard.kde.org/r/118604/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Victor Dodon
>
>
