Bug 391002 - KPopupAccelManager::setMenuEntries() is unconditionally setting QAction::setIconText()
Summary: KPopupAccelManager::setMenuEntries() is unconditionally setting QAction::setI...
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kwidgetsaddons
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: 5.43.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Christoph Feck
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-24 11:46 UTC by Christian Ehrlicher
Modified: 2018-03-30 00:18 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Ehrlicher 2018-02-24 11:46:04 UTC
While investigating https://bugreports.qt.io/browse/QTBUG-65771 I found out that KF5 is setting QAction::setIconText() for no reason in KPopupAccelManager::setMenuEntries(). This breaks the feature that QAction::iconText() is returning text() when the iconText is empty and therefore the text in the toolbar is not updated as expected when running inside KF% environment (see testcase in Qt bugreport).
Comment 1 Christoph Feck 2018-02-24 12:13:34 UTC
Thanks Christian for the investigation. Do I understand it correctly, that moving the setIconText() call into the 'if' (after line 793) is sufficient to fix the issue?
Comment 3 Christian Ehrlicher 2018-02-24 12:34:01 UTC
When you set the iconText only when the text is changed it would at least fix the bug report I would guess. The problem is that it's unknown if the iconText is set programatically or not and even watching for QEvent::ActionChanged does not really help... 
btw: The change was introduced with https://phabricator.kde.org/D7964
Comment 4 Christoph Feck 2018-03-15 02:35:49 UTC
https://phabricator.kde.org/D11346
Comment 5 Christoph Feck 2018-03-30 00:18:19 UTC
Git commit 7fe64f532d381bb43ecc03fa6d94e59f3c30ba1d by Christoph Feck.
Committed on 30/03/2018 at 00:18.
Pushed by cfeck into branch 'master'.

[KAcceleratorManager] Only set iconText() if actually changed

QAction::setIconText() breaks the feature that QAction::iconText()
is returning text() when the iconText is empty.

Reviewed by Christian Ehrlicher

Differential Revision: https://phabricator.kde.org/D11346

M  +3    -1    src/kacceleratormanager.cpp

https://commits.kde.org/kwidgetsaddons/7fe64f532d381bb43ecc03fa6d94e59f3c30ba1d