From ec8952cec9b44e7ab77c5450d8d6fb6579c21555 Mon Sep 17 00:00:00 2001
From: Mitch Curtis <mitch.curtis@qt.io>
Date: Mon, 30 Jul 2018 14:28:44 +0200
Subject: Fix Menu not being dismissed when the triggered item disables itself

Problem

Consider the following code:

    Menu {
        title: "Menu"

        Action {
            text: "Item"
            onTriggered: enabled = false
        }
    }

A MenuItem (AbstractButton) is created for the Action, and when it is
clicked, this function is called:

    void QQuickAbstractButtonPrivate::trigger()
    {
        Q_Q(QQuickAbstractButton);
        if (action && action->isEnabled())
            QQuickActionPrivate::get(action)->trigger(q, false);
        else if (effectiveEnable)
            emit q->clicked();
    }

QQuickActionPrivate::get(action)->trigger(q, false) results in this
function being called:

    void QQuickAbstractButtonPrivate::click()
    {
        Q_Q(QQuickAbstractButton);
        if (effectiveEnable)
            emit q->clicked();
    }

Since the action (and hence the menu item) was disabled in the signal
handler, the effectiveEnable check fails and clicked() is not emitted.
This causes the menu to not be dismissed.

Solution

Before calling QQuickActionPrivate::get(action)->trigger(), store
the button's enabled state. If triggering the action causes the action
to be disabled (due to the signal handler), we can then choose whether
or not we emit QQuickAbstractButton::clicked(). Specifically, we emit
clicked() if:

    - we were enabled before triggering the action, and
    - we have no associated action, or it's no longer enabled

Task-number: QTBUG-69682
Change-Id: Ib4e3c313b776decc74089a6beffe415605c430be
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
---
 tests/auto/qquickmenu/tst_qquickmenu.cpp | 70 ++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

(limited to 'tests/auto/qquickmenu/tst_qquickmenu.cpp')

diff --git a/tests/auto/qquickmenu/tst_qquickmenu.cpp b/tests/auto/qquickmenu/tst_qquickmenu.cpp
index b46b8781..a5780103 100644
--- a/tests/auto/qquickmenu/tst_qquickmenu.cpp
+++ b/tests/auto/qquickmenu/tst_qquickmenu.cpp
@@ -86,6 +86,8 @@ private slots:
     void addRemoveSubMenus();
     void scrollable_data();
     void scrollable();
+    void disableWhenTriggered_data();
+    void disableWhenTriggered();
 };
 
 void tst_QQuickMenu::defaults()
@@ -1336,6 +1338,74 @@ void tst_QQuickMenu::scrollable()
     QCOMPARE(contentItem->property("interactive").toBool(), true);
 }
 
+void tst_QQuickMenu::disableWhenTriggered_data()
+{
+    QTest::addColumn<int>("menuItemIndex");
+    QTest::addColumn<int>("subMenuItemIndex");
+
+    QTest::addRow("Action") << 0 << -1;
+    QTest::addRow("MenuItem with Action") << 1 << -1;
+    QTest::addRow("MenuItem with Action declared outside menu") << 2 << -1;
+    QTest::addRow("MenuItem with no Action") << 3 << -1;
+
+    QTest::addRow("Sub-Action") << 4 << 0;
+    QTest::addRow("Sub-MenuItem with Action declared inside") << 4 << 1;
+    QTest::addRow("Sub-MenuItem with Action declared outside menu") << 4 << 2;
+    QTest::addRow("Sub-MenuItem with no Action") << 4 << 3;
+}
+
+// Tests that the menu is dismissed when a menu item sets "enabled = false" in onTriggered().
+void tst_QQuickMenu::disableWhenTriggered()
+{
+    if ((QGuiApplication::platformName() == QLatin1String("offscreen"))
+        || (QGuiApplication::platformName() == QLatin1String("minimal")))
+        QSKIP("Mouse hovering not functional on offscreen/minimal platforms");
+
+    QFETCH(int, menuItemIndex);
+    QFETCH(int, subMenuItemIndex);
+
+    QQuickApplicationHelper helper(this, QLatin1String("disableWhenTriggered.qml"));
+    QQuickWindow *window = helper.window;
+    window->show();
+    QVERIFY(QTest::qWaitForWindowActive(window));
+
+    QQuickMenu *menu = window->findChild<QQuickMenu*>("Menu");
+    QVERIFY(menu);
+
+    menu->open();
+    QVERIFY(menu->isVisible());
+
+    QPointer<QQuickMenuItem> menuItem = qobject_cast<QQuickMenuItem*>(menu->itemAt(menuItemIndex));
+    QVERIFY(menuItem);
+
+    if (subMenuItemIndex == -1) {
+        // Click a top-level menu item.
+        QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier,
+            menuItem->mapToScene(QPointF(menuItem->width() / 2, menuItem->height() / 2)).toPoint());
+        QCOMPARE(menuItem->isEnabled(), false);
+        QVERIFY(!menu->isVisible());
+    } else {
+        // Click a sub-menu item.
+        QPointer<QQuickMenu> subMenu = menuItem->subMenu();
+        QVERIFY(subMenu);
+
+        QPointer<QQuickMenuItem> subMenuItem = qobject_cast<QQuickMenuItem*>(subMenu->itemAt(subMenuItemIndex));
+        QVERIFY(subMenuItem);
+
+        // First, open the sub-menu.
+        QTest::mouseMove(window, menuItem->mapToScene(QPoint(1, 1)).toPoint());
+        QTRY_VERIFY(subMenu->isVisible());
+        QVERIFY(menuItem->isHovered());
+        QTRY_VERIFY(subMenu->contentItem()->property("contentHeight").toReal() > 0.0);
+
+        // Click the item.
+        QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier,
+            subMenuItem->mapToScene(QPointF(subMenuItem->width() / 2, subMenuItem->height() / 2)).toPoint());
+        QCOMPARE(subMenuItem->isEnabled(), false);
+        QVERIFY(!menu->isVisible());
+    }
+}
+
 QTEST_MAIN(tst_QQuickMenu)
 
 #include "tst_qquickmenu.moc"
-- 
cgit v1.2.3