<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4403/">http://reviewboard.kde.org/r/4403/</a>
</td>
</tr>
</table>
<br />
<pre>I'll not that KIconLoader is not "incorrect" in that the icon-theme-spec does specify that hicolor should be searched if the desired icon does not exist in a theme. shared-mime-info kind of overrides that by saying that if no specific icon is located, to search for a generic fallback icon, but that should only be mime icons.
One question is what do we do if hicolor happens to have an exact match of the required size, do we accept hicolor or try to fallback to generic? I would say accept hicolor's more specific video-mp4 in preference to video-x-generic, but that could look ugly in several themes.</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="/r/4403/diff/1/?file=29183#file29183line932" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/kdeui/icons/kiconloader.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; white-space: pre-wrap; word-wrap: break-word;">K3Icon KIconLoaderPrivate::findMatchingIconWithGenericFallbacks(const QString& name, int size) const</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; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">910</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em">Be careful with this, I'm pretty sure this code path can be run for any non-User icon, not just mime icons, so doing this replace would break icons under subdirectories that are currently found.</pre>
</div>
<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="/r/4403/diff/1/?file=29183#file29183line1068" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/kdeui/icons/kiconloader.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;">K3Icon KIconLoaderPrivate::findMatchingIcon(const QString& name, int size) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;">K3Icon KIconLoaderPrivate::findMatchingIcon(const QString& _name, int size) const</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; white-space: pre-wrap; word-wrap: break-word;"></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1044</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"> <span class="p">}</span> <span class="k">else</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em">Is there a reason to move generic fallback support into findMatchingIcon where it must be used for all icons? KIconLoader is plenty slow as it currently stands, I'm not sure this would help matters.</pre>
</div>
<br />
<p>- Michael</p>
<br />
<p>On June 20th, 2010, 9:18 a.m., Aurélien Gâteau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs and Rafael Fernández López.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated 2010-06-20 09:18:37</i></p>
<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;">When loading an icon for video-mp4, KIconLoader incorrectly returns "video.png" from Hicolor, instead of returning "video-x-generic.png" from Oxygen. Since Hicolor only has a 16x16 version, this results in blurry icons in Dolphin or Gwenview when icons are bigger than 16x16.
This is because the current code fallbacks to Hicolor before looking for a "-x-generic" version.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">- Added a unit-test entry to check we get video-x-generic.
- Icons are no longer blurry after the patch.</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>trunk/KDE/kdelibs/kdeui/icons/kiconloader.cpp <span style="color: grey">(1140302)</span></li>
<li>trunk/KDE/kdelibs/kdeui/tests/kiconloader_unittest.cpp <span style="color: grey">(1140302)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4403/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>