QtMenu*: Use QtAction as private implementation

This avoids needing to duplicate some logic, in particular icon
source and name changes triggering icon updates.

Change-Id: Id8b34fa365db6d31eb6855f690c13073fd589a2c
Reviewed-by: Jens Bache-Wiig <jens.bache-wiig@digia.com>

7 files changed, 186 insertions, 182 deletions

diff --git a/src/controls/ComboBox.qml b/src/controls/ComboBox.qml
index 5086b9ec..d96c15cc 100644
--- a/src/controls/ComboBox.qml
+++ b/src/controls/ComboBox.qml
@@ -149,8 +149,8 @@ Control {
         __visualItem: comboBox
 
         function finalizeItem(item) {
-            item.action.checkable = true
-            item.action.exclusiveGroup = eg
+            item.checkable = true
+            item.exclusiveGroup = eg
         }
 
         function show() {
diff --git a/src/controls/ContextMenu.qml b/src/controls/ContextMenu.qml
index 68c855a6..ab359743 100644
--- a/src/controls/ContextMenu.qml
+++ b/src/controls/ContextMenu.qml
@@ -50,11 +50,6 @@ Menu { // Move to private
 
     Component.onCompleted: if (model !== undefined) rebuildMenu()
 
-    onSelectedIndexChanged: {
-        if (0 <= selectedIndex && selectedIndex < items.length)
-            items[selectedIndex].triggered()
-    }
-
     function rebuildMenu()
     {
         clear();
diff --git a/src/controls/qtmenu.cpp b/src/controls/qtmenu.cpp
index b188775b..75e1fbe2 100644
--- a/src/controls/qtmenu.cpp
+++ b/src/controls/qtmenu.cpp
@@ -154,11 +154,11 @@ QtMenu::~QtMenu()
     delete m_platformMenu;
 }
 
-void QtMenu::setText(const QString &text)
+void QtMenu::updateText()
 {
-    QtMenuText::setText(text);
     if (m_platformMenu)
         m_platformMenu->setText(this->text());
+    QtMenuText::updateText();
 }
 
 void QtMenu::setMinimumWidth(int w)
diff --git a/src/controls/qtmenu_p.h b/src/controls/qtmenu_p.h
index a66880cc..887fdd72 100644
--- a/src/controls/qtmenu_p.h
+++ b/src/controls/qtmenu_p.h
@@ -123,6 +123,7 @@ public:
 
 
 protected Q_SLOTS:
+    void updateText();
     void windowVisibleChanged(bool);
     void updateSelectedIndex();
 
diff --git a/src/controls/qtmenuitem.cpp b/src/controls/qtmenuitem.cpp
index e7878f25..6f438814 100644
--- a/src/controls/qtmenuitem.cpp
+++ b/src/controls/qtmenuitem.cpp
@@ -126,67 +126,91 @@ QtMenuSeparator::QtMenuSeparator(QObject *parent)
 }
 
 QtMenuText::QtMenuText(QObject *parent)
-    : QtMenuBase(parent), m_enabled(true)
-{ }
+    : QtMenuBase(parent), m_action(new QtAction(this))
+{
+    connect(m_action, SIGNAL(enabledChanged()), this, SLOT(updateEnabled()));
+    connect(m_action, SIGNAL(textChanged()), this, SLOT(updateText()));
+    connect(m_action, SIGNAL(iconNameChanged()), this, SLOT(updateIcon()));
+    connect(m_action, SIGNAL(iconNameChanged()), this, SIGNAL(iconNameChanged()));
+    connect(m_action, SIGNAL(iconSourceChanged()), this, SLOT(updateIcon()));
+    connect(m_action, SIGNAL(iconSourceChanged()), this, SIGNAL(iconSourceChanged()));
+}
 
 QtMenuText::~QtMenuText()
-{ }
+{
+    delete m_action;
+}
 
-void QtMenuText::setParentMenu(QtMenu *parentMenu)
+bool QtMenuText::enabled() const
 {
-    QtMenuBase::setParentMenu(parentMenu);
-    if (qobject_cast<QtMenuItem *>(this))
-        connect(this, SIGNAL(triggered()), parentMenu, SLOT(updateSelectedIndex()));
+    return action()->isEnabled();
 }
 
-void QtMenuText::setEnabled(bool enabled)
+void QtMenuText::setEnabled(bool e)
 {
-    if (enabled != m_enabled) {
-        m_enabled = enabled;
-        if (platformItem()) {
-            platformItem()->setEnabled(m_enabled);
-            syncWithPlatformMenu();
-        }
+    action()->setEnabled(e);
+}
 
-        emit enabledChanged();
-    }
+QString QtMenuText::text() const
+{
+    return m_action->text();
 }
 
-void QtMenuText::setText(const QString &text)
+void QtMenuText::setText(const QString &t)
 {
-    if (text != m_text) {
-        m_text = text;
-        if (platformItem()) {
-            platformItem()->setText(m_text);
-            syncWithPlatformMenu();
-        }
-        emit textChanged();
-    }
+    m_action->setText(t);
+}
+
+QUrl QtMenuText::iconSource() const
+{
+    return m_action->iconSource();
 }
 
 void QtMenuText::setIconSource(const QUrl &iconSource)
 {
-    if (iconSource != m_iconSource) {
-        m_iconSource = iconSource;
-        if (platformItem()) {
-            platformItem()->setIcon(icon());
-            syncWithPlatformMenu();
-        }
+    m_action->setIconSource(iconSource);
+}
 
-        emit iconSourceChanged();
-    }
+QString QtMenuText::iconName() const
+{
+    return m_action->iconName();
 }
 
 void QtMenuText::setIconName(const QString &iconName)
 {
-    if (iconName != m_iconName) {
-        m_iconName = iconName;
-        if (platformItem()) {
-            platformItem()->setIcon(icon());
-            syncWithPlatformMenu();
-        }
-        emit iconNameChanged();
+    m_action->setIconName(iconName);
+}
+
+QIcon QtMenuText::icon() const
+{
+    return m_action->icon();
+}
+
+void QtMenuText::updateText()
+{
+    if (platformItem()) {
+        platformItem()->setText(text());
+        syncWithPlatformMenu();
+    }
+    emit textChanged();
+}
+
+void QtMenuText::updateEnabled()
+{
+    if (platformItem()) {
+        platformItem()->setEnabled(enabled());
+        syncWithPlatformMenu();
     }
+    emit enabledChanged();
+}
+
+void QtMenuText::updateIcon()
+{
+    if (platformItem()) {
+        platformItem()->setIcon(icon());
+        syncWithPlatformMenu();
+    }
+    emit __iconChanged();
 }
 
 /*!
@@ -232,7 +256,7 @@ void QtMenuText::setIconName(const QString &iconName)
 /*!
     \qmlproperty string MenuItem::text
 
-    Text for the menu item.
+    Text for the menu item. Can be overridden after setting \l action.
 
     Mnemonics are supported by prefixing the shortcut letter with \&.
     For instance, \c "\&Open" will bind the \c Alt-O shortcut to the
@@ -249,6 +273,7 @@ void QtMenuText::setIconName(const QString &iconName)
     \qmlproperty url MenuItem::iconSource
 
     Sets the icon file or resource url for the \l MenuItem icon.
+    Can be overridden after setting \l action.
 
     \sa iconName, Action::iconSource
 */
@@ -257,7 +282,8 @@ void QtMenuText::setIconName(const QString &iconName)
     \qmlproperty string MenuItem::iconName
 
     Sets the icon name for the \l MenuItem icon. This will pick the icon
-    with the given name from the current theme.
+    with the given name from the current theme. Can be overridden after
+    setting \l action.
 
     \sa iconSource, Action::iconName
 */
@@ -321,7 +347,8 @@ void QtMenuText::setIconName(const QString &iconName)
     \qmlproperty Action MenuItem::action
 
     The action bound to this menu item. Setting this property to a valid
-    \l Action will override all the menu item's properties except \l text.
+    \l Action will override all the menu item's properties except \l text,
+    \l iconSource, and \l iconName.
 
     In addition, the menu item \c triggered() and \c toggled() signals will not be emitted.
     Instead, the action \c triggered() and \c toggled() signals will be.
@@ -330,40 +357,50 @@ void QtMenuText::setIconName(const QString &iconName)
 */
 
 QtMenuItem::QtMenuItem(QObject *parent)
-    : QtMenuText(parent), m_action(0)
-{ }
+    : QtMenuText(parent), m_boundAction(0)
+{
+    connect(action(), SIGNAL(triggered()), this, SIGNAL(triggered()));
+    connect(action(), SIGNAL(toggled(bool)), this, SLOT(updateChecked()));
+    if (platformItem())
+        connect(platformItem(), SIGNAL(activated()), this, SLOT(trigger()));
+}
 
 QtMenuItem::~QtMenuItem()
 {
-    unbindFromAction(m_action);
+    unbindFromAction(m_boundAction);
+    if (platformItem())
+        disconnect(platformItem(), SIGNAL(activated()), this, SLOT(trigger()));
 }
 
-void QtMenuItem::bindToAction(QtAction *action)
+void QtMenuItem::setParentMenu(QtMenu *parentMenu)
 {
-    m_action = action;
+    QtMenuText::setParentMenu(parentMenu);
+    connect(this, SIGNAL(triggered()), parentMenu, SLOT(updateSelectedIndex()));
+}
 
-    if (platformItem()) {
-        connect(platformItem(), SIGNAL(activated()), m_action, SLOT(trigger()));
-    }
+void QtMenuItem::bindToAction(QtAction *action)
+{
+    m_boundAction = action;
 
-    connect(m_action, SIGNAL(destroyed(QObject*)), this, SLOT(unbindFromAction(QObject*)));
+    connect(m_boundAction, SIGNAL(destroyed(QObject*)), this, SLOT(unbindFromAction(QObject*)));
 
-    connect(m_action, SIGNAL(triggered()), this, SIGNAL(triggered()));
-    connect(m_action, SIGNAL(toggled(bool)), this, SLOT(updateChecked()));
-    connect(m_action, SIGNAL(exclusiveGroupChanged()), this, SIGNAL(exclusiveGroupChanged()));
-    connect(m_action, SIGNAL(enabledChanged()), this, SLOT(updateEnabled()));
-    connect(m_action, SIGNAL(textChanged()), this, SLOT(updateText()));
-    connect(m_action, SIGNAL(shortcutChanged(QString)), this, SLOT(updateShortcut()));
-    connect(m_action, SIGNAL(checkableChanged()), this, SIGNAL(checkableChanged()));
-    connect(m_action, SIGNAL(iconNameChanged()), this, SLOT(updateIconName()));
-    connect(m_action, SIGNAL(iconSourceChanged()), this, SLOT(updateIconSource()));
+    connect(m_boundAction, SIGNAL(triggered()), this, SIGNAL(triggered()));
+    connect(m_boundAction, SIGNAL(toggled(bool)), this, SLOT(updateChecked()));
+    connect(m_boundAction, SIGNAL(exclusiveGroupChanged()), this, SIGNAL(exclusiveGroupChanged()));
+    connect(m_boundAction, SIGNAL(enabledChanged()), this, SLOT(updateEnabled()));
+    connect(m_boundAction, SIGNAL(textChanged()), this, SLOT(updateText()));
+    connect(m_boundAction, SIGNAL(shortcutChanged(QString)), this, SLOT(updateShortcut()));
+    connect(m_boundAction, SIGNAL(checkableChanged()), this, SIGNAL(checkableChanged()));
+    connect(m_boundAction, SIGNAL(iconNameChanged()), this, SLOT(updateIcon()));
+    connect(m_boundAction, SIGNAL(iconNameChanged()), this, SIGNAL(iconNameChanged()));
+    connect(m_boundAction, SIGNAL(iconSourceChanged()), this, SLOT(updateIcon()));
+    connect(m_boundAction, SIGNAL(iconSourceChanged()), this, SIGNAL(iconSourceChanged()));
 
-    if (m_action->parent() != this) {
+    if (m_boundAction->parent() != this) {
         updateText();
         updateShortcut();
         updateEnabled();
-        updateIconName();
-        updateIconSource();
+        updateIcon();
         if (checkable())
             updateChecked();
     }
@@ -374,17 +411,13 @@ void QtMenuItem::unbindFromAction(QObject *o)
     if (!o)
         return;
 
-    if (o == m_action)
-        m_action = 0;
+    if (o == m_boundAction)
+        m_boundAction = 0;
 
     QtAction *action = qobject_cast<QtAction *>(o);
     if (!action)
         return;
 
-    if (platformItem()) {
-        disconnect(platformItem(), SIGNAL(activated()), action, SLOT(trigger()));
-    }
-
     disconnect(action, SIGNAL(destroyed(QObject*)), this, SLOT(unbindFromAction(QObject*)));
 
     disconnect(action, SIGNAL(triggered()), this, SIGNAL(triggered()));
@@ -394,27 +427,29 @@ void QtMenuItem::unbindFromAction(QObject *o)
     disconnect(action, SIGNAL(textChanged()), this, SLOT(updateText()));
     disconnect(action, SIGNAL(shortcutChanged(QString)), this, SLOT(updateShortcut()));
     disconnect(action, SIGNAL(checkableChanged()), this, SIGNAL(checkableChanged()));
-    disconnect(action, SIGNAL(iconNameChanged()), this, SLOT(updateIconName()));
-    disconnect(action, SIGNAL(iconSourceChanged()), this, SLOT(updateIconSource()));
+    disconnect(action, SIGNAL(iconNameChanged()), this, SLOT(updateIcon()));
+    disconnect(action, SIGNAL(iconNameChanged()), this, SIGNAL(iconNameChanged()));
+    disconnect(action, SIGNAL(iconSourceChanged()), this, SLOT(updateIcon()));
+    disconnect(action, SIGNAL(iconSourceChanged()), this, SIGNAL(iconSourceChanged()));
 }
 
-QtAction *QtMenuItem::action()
+QtAction *QtMenuItem::action() const
 {
-    if (!m_action)
-        bindToAction(new QtAction(this));
-    return m_action;
+    if (m_boundAction)
+        return m_boundAction;
+    return QtMenuText::action();
 }
 
-void QtMenuItem::setAction(QtAction *a)
+void QtMenuItem::setBoundAction(QtAction *a)
 {
-    if (a == m_action)
+    if (a == m_boundAction)
         return;
 
-    if (m_action) {
-        if (m_action->parent() == this)
-            delete m_action;
+    if (m_boundAction) {
+        if (m_boundAction->parent() == this)
+            delete m_boundAction;
         else
-            unbindFromAction(m_action);
+            unbindFromAction(m_boundAction);
     }
 
     bindToAction(a);
@@ -423,27 +458,45 @@ void QtMenuItem::setAction(QtAction *a)
 
 QString QtMenuItem::text() const
 {
-    return m_action ? m_action->text() : QString();
+    QString ownText = QtMenuText::text();
+    if (!ownText.isEmpty())
+        return ownText;
+    return m_boundAction ? m_boundAction->text() : QString();
 }
 
-void QtMenuItem::setText(const QString &text)
+QUrl QtMenuItem::iconSource() const
 {
-    action()->setText(text);
+    QUrl ownIconSource = QtMenuText::iconSource();
+    if (!ownIconSource.isEmpty())
+        return ownIconSource;
+    return m_boundAction ? m_boundAction->iconSource() : QUrl();
 }
 
-void QtMenuItem::updateText()
+QString QtMenuItem::iconName() const
 {
-    QtMenuText::setText(text());
+    QString ownIconName = QtMenuText::iconName();
+    if (!ownIconName.isEmpty())
+        return ownIconName;
+    return m_boundAction ? m_boundAction->iconName() : QString();
+}
+
+QIcon QtMenuItem::icon() const
+{
+    QIcon ownIcon = QtMenuText::icon();
+    if (!ownIcon.isNull())
+        return ownIcon;
+    return m_boundAction ? m_boundAction->icon() : QIcon();
 }
 
 QString QtMenuItem::shortcut() const
 {
-    return m_action ? m_action->shortcut() : QString();
+    return action()->shortcut();
 }
 
 void QtMenuItem::setShortcut(const QString &shortcut)
 {
-    action()->setShortcut(shortcut);
+    if (!m_boundAction)
+        action()->setShortcut(shortcut);
 }
 
 void QtMenuItem::updateShortcut()
@@ -457,22 +510,24 @@ void QtMenuItem::updateShortcut()
 
 bool QtMenuItem::checkable() const
 {
-    return m_action ? m_action->isCheckable() : false;
+    return action()->isCheckable();
 }
 
 void QtMenuItem::setCheckable(bool checkable)
 {
-    action()->setCheckable(checkable);
+    if (!m_boundAction)
+        action()->setCheckable(checkable);
 }
 
 bool QtMenuItem::checked() const
 {
-    return m_action ? m_action->isChecked() : false;
+    return action()->isChecked();
 }
 
 void QtMenuItem::setChecked(bool checked)
 {
-    action()->setChecked(checked);
+    if (!m_boundAction)
+        action()->setChecked(checked);
 }
 
 void QtMenuItem::updateChecked()
@@ -487,70 +542,24 @@ void QtMenuItem::updateChecked()
 
 QtExclusiveGroup *QtMenuItem::exclusiveGroup() const
 {
-    return m_action ? m_action->exclusiveGroup() : 0;
+    return action()->exclusiveGroup();
 }
 
 void QtMenuItem::setExclusiveGroup(QtExclusiveGroup *eg)
 {
-    action()->setExclusiveGroup(eg);
-}
-
-bool QtMenuItem::enabled() const
-{
-    return m_action ? m_action->isEnabled() : false;
+    if (!m_boundAction)
+        action()->setExclusiveGroup(eg);
 }
 
 void QtMenuItem::setEnabled(bool enabled)
 {
-    action()->setEnabled(enabled);
-}
-
-void QtMenuItem::updateEnabled()
-{
-    QtMenuText::setEnabled(enabled());
-}
-
-QUrl QtMenuItem::iconSource() const
-{
-    return m_action ? m_action->iconSource() : QUrl();
-}
-
-void QtMenuItem::setIconSource(const QUrl &iconSource)
-{
-    action()->setIconSource(iconSource);
-}
-
-void QtMenuItem::updateIconSource()
-{
-    QtMenuText::setIconSource(iconSource());
-}
-
-QString QtMenuItem::iconName() const
-{
-    return m_action ? m_action->iconName() : QString();
-}
-
-void QtMenuItem::setIconName(const QString &iconName)
-{
-    action()->setIconName(iconName);
-}
-
-void QtMenuItem::updateIconName()
-{
-    QtMenuText::setIconName(iconName());
-}
-
-QIcon QtMenuItem::icon() const
-{
-    return m_action ? m_action->icon() : QtMenuText::icon();
+    if (!m_boundAction)
+        action()->setEnabled(enabled);
 }
 
 void QtMenuItem::trigger()
 {
-    if (m_action)
-        m_action->trigger();
-    else
-        emit triggered();
+    action()->trigger();
 }
 
 QT_END_NAMESPACE
diff --git a/src/controls/qtmenuitem_p.h b/src/controls/qtmenuitem_p.h
index 8644a490..b3855254 100644
--- a/src/controls/qtmenuitem_p.h
+++ b/src/controls/qtmenuitem_p.h
@@ -124,27 +124,30 @@ public:
     QtMenuText(QObject *parent = 0);
     ~QtMenuText();
 
-    virtual bool enabled() const { return m_enabled; }
+    bool enabled() const;
     virtual void setEnabled(bool enabled);
 
-    virtual QString text() const { return m_text; }
-    virtual void setText(const QString &text);
-
-    virtual QUrl iconSource() const { return m_iconSource; }
-    virtual void setIconSource(const QUrl &icon);
-    virtual QString iconName() const { return m_iconName; }
-    virtual void setIconName(const QString &icon);
+    virtual QString text() const;
+    void setText(const QString &text);
 
-    void setParentMenu(QtMenu *parentMenu);
+    virtual QUrl iconSource() const;
+    void setIconSource(const QUrl &icon);
+    virtual QString iconName() const;
+    void setIconName(const QString &icon);
 
-    virtual QIcon icon() const { return QIcon(); } // TODO
     QVariant iconVariant() const { return QVariant(icon()); }
 
+protected:
+    virtual QIcon icon() const;
+    virtual QtAction *action() const { return m_action; }
+
+protected Q_SLOTS:
+    virtual void updateText();
+    void updateEnabled();
+    void updateIcon();
+
 private:
-    QString m_text;
-    bool m_enabled;
-    QUrl m_iconSource;
-    QString m_iconName;
+    QtAction *m_action;
 };
 
 class QtMenuItem: public QtMenuText
@@ -154,7 +157,7 @@ class QtMenuItem: public QtMenuText
     Q_PROPERTY(bool checked READ checked WRITE setChecked NOTIFY toggled)
     Q_PROPERTY(QtExclusiveGroup *exclusiveGroup READ exclusiveGroup WRITE setExclusiveGroup NOTIFY exclusiveGroupChanged)
     Q_PROPERTY(QString shortcut READ shortcut WRITE setShortcut NOTIFY shortcutChanged)
-    Q_PROPERTY(QtAction *action READ action WRITE setAction NOTIFY actionChanged)
+    Q_PROPERTY(QtAction *action READ boundAction WRITE setBoundAction NOTIFY actionChanged)
 
 public Q_SLOTS:
     void trigger();
@@ -172,19 +175,15 @@ public:
     QtMenuItem(QObject *parent = 0);
     ~QtMenuItem();
 
-    bool enabled() const;
     void setEnabled(bool enabled);
 
     QString text() const;
-    void setText(const QString &text);
 
     QUrl iconSource() const;
-    void setIconSource(const QUrl &icon);
     QString iconName() const;
-    void setIconName(const QString &icon);
 
-    QtAction *action();
-    void setAction(QtAction *a);
+    QtAction *boundAction() { return m_boundAction; }
+    void setBoundAction(QtAction *a);
 
     QString shortcut() const;
     void setShortcut(const QString &shortcut);
@@ -198,20 +197,20 @@ public:
     QtExclusiveGroup *exclusiveGroup() const;
     void setExclusiveGroup(QtExclusiveGroup *);
 
-    QIcon icon() const;
+    void setParentMenu(QtMenu *parentMenu);
 
 protected Q_SLOTS:
-    virtual void updateText();
     void updateShortcut();
     void updateChecked();
-    void updateEnabled();
-    void updateIconName();
-    void updateIconSource();
     void bindToAction(QtAction *action);
     void unbindFromAction(QObject *action);
 
+protected:
+    QIcon icon() const;
+    QtAction *action() const;
+
 private:
-    QtAction *m_action;
+    QtAction *m_boundAction;
 };
 
 QT_END_NAMESPACE
diff --git a/src/styles/Desktop/MenuStyle.qml b/src/styles/Desktop/MenuStyle.qml
index c7cda6e3..f40b42aa 100644
--- a/src/styles/Desktop/MenuStyle.qml
+++ b/src/styles/Desktop/MenuStyle.qml
@@ -89,7 +89,7 @@ Style {
             "exclusive": !!menuItem && !!menuItem["exclusiveGroup"],
             "shortcut": !!menuItem && menuItem["shortcut"] || "",
             "hasSubmenu": hasSubmenu,
-            "icon": !!menuItem && !!menuItem["action"] ? menuItem.action.__icon : null
+            "icon": !!menuItem && menuItem.__icon
         }
     }
 }