From 21f954d94dc52f36b0cbbadba09257a5d68a04d0 Mon Sep 17 00:00:00 2001 From: Kai Uwe Broulik Date: Tue, 11 Jul 2017 12:23:39 +0200 Subject: [PATCH] [PlasmaComponents Menu] Don't crash on null action You can assign a QAction as "action", this way you can just pass it e.g. plasmoid.action("configure"). However, when assigning a null action as can happen with kiosk restrictions, it would crash as it assigns m_action the nullptr but never checks for that. This patch ensures we always have a QAction, creating a new empty one, if neccessary. Also deletes our own action if an external one is assigned Differential Revision: https://phabricator.kde.org/D6608 --- .../plasmacomponents/qmenuitem.cpp | 18 +++++++++++++++++- tests/components/menu.qml | 16 +++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/declarativeimports/plasmacomponents/qmenuitem.cpp b/src/declarativeimports/plasmacomponents/qmenuitem.cpp index b91eb029a..f773a26d4 100644 --- a/src/declarativeimports/plasmacomponents/qmenuitem.cpp +++ b/src/declarativeimports/plasmacomponents/qmenuitem.cpp @@ -38,8 +38,24 @@ void QMenuItem::setAction(QAction *a) if (m_action != a) { if (m_action) { disconnect(m_action, 0, this, 0); + + if (m_action->parent() == this) { + delete m_action; + m_action = nullptr; + } } - m_action = a; + + if (a) { + m_action = a; + } else { + // don't end up with no action, create an invisible one instead + m_action = new QAction(this); + m_action->setVisible(false); + } + + setVisible(m_action->isVisible()); + setEnabled(m_action->isEnabled()); + connect(m_action, &QAction::changed, this, &QMenuItem::textChanged); connect(m_action, &QAction::changed, this, &QMenuItem::checkableChanged); connect(m_action, SIGNAL(toggled(bool)), this, SIGNAL(toggled(bool))); diff --git a/tests/components/menu.qml b/tests/components/menu.qml index 84f5a3b15..3d72a6fe3 100644 --- a/tests/components/menu.qml +++ b/tests/components/menu.qml @@ -3,7 +3,7 @@ import org.kde.plasma.components 2.0 as PlasmaComponents Rectangle { width: 600 - height: 200 + height: 250 color: "white" Flow { @@ -99,5 +99,19 @@ Rectangle { checked: true } } + + PlasmaComponents.Button { + text: "Don't crash on null MenuItem action" + onClicked: noActionCrashMenu.open(0, height) + + PlasmaComponents.Menu { + id: noActionCrashMenu + + PlasmaComponents.MenuItem { text: "This is an item" } + PlasmaComponents.MenuItem { text: "Below me should NOT be an empty item"} + PlasmaComponents.MenuItem { action: null } + PlasmaComponents.MenuItem { text: "I am not empty" } + } + } } }