Review Request: PreviewJob supports webpages

[David Faure]

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



kioslave/thumbnail/htmlcreator.cpp
<http://git.reviewboard.kde.org/r/104821/#comment10399>

    Unrelated to your patch, but this QUrl(...) wrapping is unnecessary, KUrl is a QUrl anyway.



kioslave/thumbnail/htmlcreator.cpp
<http://git.reviewboard.kde.org/r/104821/#comment10398>

    So if you have a local HTML file in your bookmarks, it will stand out as being in a different format than all other previews? This seems very strange and inconsistent.
    
    Sounds to me like the caller should specify the requested size. IIRC this is already available in the PreviewJob API, too. Please look into that, it would be much better than hardcoded sizes inside the thumbnailer...



kioslave/thumbnail/htmlthumbnail.desktop
<http://git.reviewboard.kde.org/r/104821/#comment10400>

    +https?
    
    In fact this should say X-KDE-Protocol=KIO, since any kioslave would work. But I'm not sure PreviewJob supports the "KIO" special case (krun.cpp does, though).
    
    Feel free to ignore this for now, unless it works out of the box with "=KIO".


- David Faure


On May 2, 2012, 10:55 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104821/
> -----------------------------------------------------------
> 
> (Updated May 2, 2012, 10:55 a.m.)
> 
> 
> Review request for KDE Runtime and Dawit Alemayehu.
> 
> 
> Description
> -------
> 
> Make htmlcreator thumbnailer plugin support remote URLs
> 
> Requesting thumbnails for URLs has been added to kdelibs in 4.7. This patch makes the webcreator thumbnailer plugin support remote URLs as well.
> 
> We've been using a similar webcreator in Plasma Active, with this patch, our use cases would be covered by stock KDE.
> 
> 
> Diffs
> -----
> 
>   kioslave/thumbnail/htmlcreator.cpp afbcea2 
>   kioslave/thumbnail/htmlthumbnail.desktop 9cc4e02 
> 
> Diff: http://git.reviewboard.kde.org/r/104821/diff/
> 
> 
> Testing
> -------
> 
> Used plugin through Plasma's preview engine for URLs, thumbnails of local html files also still work (tested through Dolphin).
> 
> I'm getting all-white images sometimes, the check for mainFrame()'s contentSize() mitigates this partly, but it still happens. As that's not a regression for local code, I'd like to get this patch merged. (When the error happens, loadFinished() still returns true, so it isn't caught by the m_loadedOk guard.) If anyone has suggestions how to handle these kinds of "silent failures", they would be most welcome :)
> 
> 
> Screenshots
> -----------
> 
> previews for bookmarks
>   http://git.reviewboard.kde.org/r/104821/s/554/
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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