<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119551/">https://git.reviewboard.kde.org/r/119551/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Nice detailed analysis, sounds all correct to me.</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/119551/diff/1/?file=294569#file294569line685" style="color: black; font-weight: bold; text-decoration: underline;">src/filewidgets/kfilepreviewgenerator.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KFilePreviewGenerator::Private::addToPreviewQueue(const KFileItem &item, const QPixmap &pixmap)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">685</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">m_pendingItems</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This if () isn't useful, the for loop below will be a no-op if the list is empty.</p></pre>
</div>
</div>
<br />
<p>- David Faure</p>
<br />
<p>On July 31st, 2014, 6:04 a.m. UTC, Eike Hein wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Bhushan Shah and David Faure.</div>
<div>By Eike Hein.</div>
<p style="color: grey;"><i>Updated July 31, 2014, 6:04 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=337815">337815</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDirModel, KDirLister and KFilePreviewGenerator interact as follows:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/filewidgets/kfilepreviewgenerator.cpp <span style="color: grey">(274bdf2)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119551/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>