<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/126757/">https://git.reviewboard.kde.org/r/126757/</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;">Ship It!</pre>
<br />
<p>- Aleix Pol Gonzalez</p>
<br />
<p>On January 15th, 2016, 4:30 p.m. CET, René J.V. Bertin 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 Software on Mac OS X and KDevelop.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Jan. 15, 2016, 4:30 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">There are platforms on which <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QIcon::fromTheme()</code> will almost always return the fallback icon (empty by default) because icon themes are not supported "out of the box" by Qt.
If however the application was endowed with an icon at build time, chances are that at least some of the targets of the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">setWindowIcon()</code> call have inherited the application on the that platform.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I propose to preserve that icon rather than replace it with an empty icon when a lookup-from-theme fails, by providing an explicit fallback to all those <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QIcon::fromTheme()</code> calls:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">setWindowIcon( QIcon::fromTheme(name, windowIcon()) )</code></p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I haven't tried to verify to what extent this indeed has an effect for all calls. I think however that it's probably best to add this explicit fallback systematically.</p></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; 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;">On OS X 10.9.5 and Kubuntu 14.04, both with Qt 5.5.1 and KF5 Frameworks 5.17.0 installed into /opt/local .</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch has no noticeable effect on my Linux system where <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QIcon::fromTheme()</code> works normally. On OS S, KDevelop will appear with a default application icon in the Dock and App Switcher when this patch isn't applied (and the plugins from the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">frameworkintegration</code> framework aren't loaded).</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>debugger/breakpoint/breakpointwidget.cpp <span style="color: grey">(df8851e)</span></li>
<li>debugger/framestack/framestackwidget.cpp <span style="color: grey">(04bf8da)</span></li>
<li>debugger/variable/variablewidget.cpp <span style="color: grey">(0bd5ab8)</span></li>
<li>documentation/documentationview.cpp <span style="color: grey">(216ee32)</span></li>
<li>plugins/classbrowser/classwidget.cpp <span style="color: grey">(4a8db5c)</span></li>
<li>plugins/contextbrowser/contextbrowserview.cpp <span style="color: grey">(d1dba72)</span></li>
<li>plugins/documentview/kdevdocumentview.cpp <span style="color: grey">(3a32636)</span></li>
<li>plugins/externalscript/externalscriptview.cpp <span style="color: grey">(a7d2e7a)</span></li>
<li>plugins/filemanager/filemanager.cpp <span style="color: grey">(1afe763)</span></li>
<li>plugins/grepview/grepoutputview.cpp <span style="color: grey">(ab991ce)</span></li>
<li>plugins/konsole/kdevkonsoleview.cpp <span style="color: grey">(de4445e)</span></li>
<li>plugins/outlineview/outlinewidget.cpp <span style="color: grey">(8a49cd6)</span></li>
<li>plugins/problemreporter/problemsview.cpp <span style="color: grey">(e4fd505)</span></li>
<li>plugins/projectmanagerview/projectmanagerview.cpp <span style="color: grey">(fd8bc02)</span></li>
<li>plugins/testview/testview.cpp <span style="color: grey">(12a9a8b)</span></li>
<li>plugins/vcschangesview/vcschangesview.cpp <span style="color: grey">(5d9b26b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126757/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>