D7954: Fix repaint loop in KToolBar

David Faure noreply at phabricator.kde.org
Sat Sep 23 20:21:23 UTC 2017


dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  OK, let's get the quick fix in, but I still think my alternative patch (removing all this code and applying http://www.davidfaure.fr/2017/0001-KAcceleratorManager-set-icon-text-on-actions-to-remo.patch instead) is a better solution.
  It works in my tests, although the discussion in https://git.reviewboard.kde.org/r/129663/ got a little confusing.

INLINE COMMENTS

> ktoolbar.cpp:1340
> +                    const QString modText = i18nc("@action:intoolbar Text label of toolbar button", "%1", text);
> +                    if (modText != text) {
> +                        tb->setText(modText);

This if() serves no purpose, setText already does this:

  void QAbstractButton::setText(const QString &text)                                                                                                                                                                      
  {
      Q_D(QAbstractButton);
      if (d->text == text)
          return;

> ktoolbar.cpp:1345
> +                    const QString modToolTip = i18nc("@info:tooltip Tooltip of toolbar button", "%1", toolTip);
> +                    if (modToolTip != toolTip) {
> +                        tb->setToolTip(modToolTip);

QWidget::setToolTip however lacks the equality check, how strange. On the other hand that just generates a ToolTipChange event, surely not the cause for the recursion here. You can keep the check, but for the record, that's not the fix.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D7954

To: mardelle, #frameworks, ilic, dfaure
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170923/7dde391b/attachment.html>


More information about the Kde-frameworks-devel mailing list