<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/126308/">https://git.reviewboard.kde.org/r/126308/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">What tells you that this is a dialog buttonbox pushbutton?</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">What happens if the button has no text?</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The bug is in QDialogButtonBox (or rather the K variant, QDialogButtonBoxPrivate::createButton() seems to incorporate the hint correctly)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was interpreted by KPushButton <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">and</em> kstyle for the hint, but the hint only covers Q'DialogButtonBox'es - there's simply no global rule for this like AA_DontShowIconsInMenus</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">=> KDialogButtonBox shall respect the hint for its buttons (there're two special creation routines).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For the rest, the platform plugin ideally picks the kdeglobals setting and exports it to the application object (dyn property?) where the style can pick it and incorporate it into its calculations (ie. if no icons are wanted and there's text or arrow, omit the icon in size calculation and painting)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"Fixing" that in deprecated KPushButton doesn't really fix anything. We'll face the mix we had, just that users of QPushButton were far less prone to pass them an icon in pre-KF5 times.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please also attach Hugo Pereira Da Costa.</p></pre>
<br />
<p>- Thomas Lübking</p>
<br />
<p>On December 10th, 2015, 9:01 p.m. UTC, 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, KDE Frameworks and Qt KDE.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Dec. 10, 2015, 9:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs4support
</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;">KF5 applications have long had a habit of drawing icons on buttons even when this feature was turned off in the user's setting. This was mostly noticeable in applications built on kdelibs4support.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It seems that the actual culprit is in Qt's QPushButton implementation (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work around it in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KPushButton::paintEvent</code>, by removing the icon (forcing it to the null icon) in the option instance, before handing off control to the painter.</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 Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have not yet verified if there are other classes where this modification would be relevant too.</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/kdeui/kpushbutton.cpp <span style="color: grey">(98534fa)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126308/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>