[KDE/Mac] Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J.V. Bertin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/
-----------------------------------------------------------

(Updated Dec. 11, 2015, 5:26 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo Pereira Da Costa, and Yichao Yu.


Changes
-------

Thomas, what exactly did you mean with "QDialogButtonBox::addButton should do correctly"? Looking at Qt's code again, I can confirm that QDB::addButton(StandardButton) is the only one invoking the private createButton method which in turn sets an icon if ButtonsHaveIcons is true. The other QDB::addButton methods simply call the private addButton method which will do a layout step by default, but I don't see where an icon would be added.

Should I understand that `style->standardIcon()` is invoked during drawing as a function of button's role?

I cannot find evidence of that in QtCurve. But if that is the case nonetheless, why are we patching KDialogButtonBox again? And how do you explain that removing the icon after a button is created has the effect it has (when the style will continue to see the button role)?


Repository: kdelibs4support


Description
-------

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.

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 `KPushButton::paintEvent`, by removing the icon (forcing it to the null icon) in the option instance, before handing off control to the painter.


Diffs (updated)
-----

  src/kdeui/kdialogbuttonbox.cpp 0f6649b 
  src/kdeui/kpushbutton.cpp 98534fa 

Diff: https://git.reviewboard.kde.org/r/126308/diff/


Testing
-------

On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .

I have not yet verified if there are other classes where this modification would be relevant too.


Thanks,

René J.V. Bertin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20151211/0c323445/attachment.html>