<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/128580/">https://git.reviewboard.kde.org/r/128580/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 3rd, 2016, 7:38 a.m. UTC, <b>David Rosca</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Why does it break for SNIs? It first tries QIcon::fromTheme and if it fails (isNull()) fallbacks to original QIcon. This patch just switched it, why does it make a difference?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The problem I see here is that if a custom icon loader is used which uses static icon name (let's say "plasma" as we have it in breeze icons), but the actual plasma.png file representing completely different icon. In this case, however, it would fail in the codepath above - being loaded either from plasma theme or svg with kiconloader.</p></pre>
</blockquote>
</blockquote>
<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;">The reason it fails, is that QIcon::fromTheme ends up returning the unknown icon. Which is not "null".
and I can't just disable it having a fallback, as maybe it originally was a string which didn't have an icon.</p></pre>
<br />
<p>- David</p>
<br />
<p>On August 3rd, 2016, 11:05 a.m. UTC, David Edmundson 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 and Plasma.</div>
<div>By David Edmundson.</div>
<p style="color: grey;"><i>Updated Aug. 3, 2016, 11:05 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</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;">Currently the code gets the icon name from the QIcon and tries to do
some Plasma theming with it.
However if that fails it then loads the QIcon::fromTheme again.
This is pointless in most cases and will break any icons that have a
custom loader (all SNIs)</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/declarativeimports/core/iconitem.cpp <span style="color: grey">(29c7f05b5df060df7b362b331f7edc412df12307)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128580/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>