Review Request: Use KFileItem::targetUrl() instead of KFileItem::url() in PreviewJob

[David Faure]

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


I'm a bit undecided about this. The concept of targetUrl is:
* url() is the url of the item we are looking at in the file view
* targetUrl() is the url that we will open when clicking on the item
This doesn't really tell us if url() or targetUrl() is better for generating the preview, this can be very dependent on the ioslave IMHO.

On the other hand, using mostLocalUrl() would definitely be correct; wouldn't that work for you?

- David


On 2010-09-03 20:27:44, Sebastian Trueg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5251/
> -----------------------------------------------------------
> 
> (Updated 2010-09-03 20:27:44)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Currently the PreviewJob uses KFileItem::url() to generate the preview for files. However, today targetUrl() is the correct one to use since other hierarchies than the file:/ one can contain local files.
> To be precise one would even have to check for UDS_LOCAL_FILE but using targetUrl() is a good start.
> Since PreviewJob is a rather big class I would like to have someone look at the patch to check if I missed any url() -> targetUrl() replacement.
> 
> 
> This addresses bug 248234.
>     https://bugs.kde.org/show_bug.cgi?id=248234
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kio/kio/previewjob.cpp 1171417 
> 
> Diff: http://svn.reviewboard.kde.org/r/5251/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100906/e92a85b9/attachment.htm>