> On May 9, 2012, 12:04 p.m., David Faure wrote:
> > kio/kio/dataprotocol.cpp, line 71
> > <http://git.reviewboard.kde.org/r/104874/diff/1/?file=63068#file63068line71>
> >
> >     Not sure the comment is still correct. "decoded"? It's not, anymore, 
> > right? It's just extracted? Or is it even the full initial URL?

Yes, it is still correct. This is after the percent encoding has been decoded.


> On May 9, 2012, 12:04 p.m., David Faure wrote:
> > kio/kio/dataprotocol.cpp, line 277
> > <http://git.reviewboard.kde.org/r/104874/diff/1/?file=63068#file63068line277>
> >
> >     Why .toUtf8()? Are we sure that this is what the receiver of the data 
> > will use, for decoding?
> >     (I mean in the real case of an application getting the data, not in the 
> > unittest)

I have used the new testcases on the old code and compared with the online 
tests. The only cases where old code was right and the string afterwards was 
displayed correctly was when it returned UTF8 strings. So returning UTF8 is the 
right thing here IMHO.


> On May 9, 2012, 12:04 p.m., David Faure wrote:
> > kio/kio/dataprotocol.cpp, line 279
> > <http://git.reviewboard.kde.org/r/104874/diff/1/?file=63068#file63068line279>
> >
> >     This comment isn't applicable anymore, it was the justification for 
> > toLocal8Bit().

Correct, will delete that.


- Rolf Eike


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


On May 6, 2012, 6:14 p.m., Rolf Eike Beer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104874/
> -----------------------------------------------------------
> 
> (Updated May 6, 2012, 6:14 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> This reworks the code that works with different character sets to actually do 
> the right thing (tm).
> 
> 
> Diffs
> -----
> 
>   kio/kio/dataprotocol.cpp e614476 
>   kio/tests/dataprotocoltest.cpp c8df132 
> 
> Diff: http://git.reviewboard.kde.org/r/104874/diff/
> 
> 
> Testing
> -------
> 
> -build whole kdelibs
> -added more testcases from http://greenbytes.de/tech/tc/datauri
> -all dataprotocol tests pass
> 
> 
> Thanks,
> 
> Rolf Eike Beer
> 
>

Reply via email to