Review Request 119551: Fix KFilePreviewGenerator not triggering model change update when determining MIME type as byproduct of unsuccessful preview job

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119551/#review63540
-----------------------------------------------------------

Ship it!


Nice detailed analysis, sounds all correct to me.


src/filewidgets/kfilepreviewgenerator.cpp
<https://git.reviewboard.kde.org/r/119551/#comment44283>

    This if () isn't useful, the for loop below will be a no-op if the list is empty.


- David Faure


On July 31, 2014, 6:04 a.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119551/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 6:04 a.m.)
> 
> 
> Review request for KDE Frameworks, Bhushan Shah and David Faure.
> 
> 
> Bugs: 337815
>     https://bugs.kde.org/show_bug.cgi?id=337815
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> KDirModel, KDirLister and KFilePreviewGenerator interact as follows:
> 
> KDirModel uses a KDirLister as its data source. KDirLister has a boolean "delayed MIME types" option. When enabled, file MIME types are not determined automatically, but only when requested explicitly via KFileItem::determineMimeType(). This does not immediately cause a model update (dataChanged), i.e. it falls to whatever entity that decides to determine the MIME type to also announce a model change once the MIME type has been resolved. Until its MIME type has been resolved, the DecorationRole for an item in the model will be a generic icon.
> 
> In practice, this falls to KFilePreviewGenerator. KFilePreviewGenerator operates in two modes: Previews on or off. With previews off, it resolves MIME types and announces model updates. With previews on, it uses KIO::PreviewJobs to generate preview thumbnails and cause model updates to be announced.
> 
> But there's an unhandled case: When a PreviewJob does not actually generate a preview thumbnail for a file item (e.g. because there's no thumbnailer supporting the MIME type, or it's broken for whatever reason), no model change is announced for it. This is despite the fact that a PreviewJob implicitly causes the item's MIME type to be resolved (it has to, to figure out the right thumbnailer plugin to query), and so even though no thumbnail has been generated, it might now be possible to go from the generic icon to a proper MIME type icon (i.e. exactly what would happen if previews were off).
> 
> The patch proposed here does the following: When all outstanding preview jobs have finished, it loops over the still-pending items (i.e. that didn't actually get a preview generated) and adds those that had their MIME types resolved to the list the "previews off" codepath uses to track items that had their MIME types resolved. The dispatch method (that causes model change annoucements) is modified to always check this list even if previews are on.
> 
> This fixes awkard behavior in views, because determining the MIME type effectively causes the DecorationRole to change but this change isn't announced by the model via dataChanged(). With this patch, it is.
> 
> 
> Diffs
> -----
> 
>   src/filewidgets/kfilepreviewgenerator.cpp 274bdf2 
> 
> Diff: https://git.reviewboard.kde.org/r/119551/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140731/7d8cb477/attachment.html>