Fix overlay event processing order for popups

It should follow the stacking order of popups.

Change-Id: Id5664496c60e99bdf0aa4b2dc6a2c94dba2c756a
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

3 files changed, 79 insertions, 14 deletions

diff --git a/src/quicktemplates2/qquickoverlay.cpp b/src/quicktemplates2/qquickoverlay.cpp
index 2e19e02d..efbc2ce5 100644
--- a/src/quicktemplates2/qquickoverlay.cpp
+++ b/src/quicktemplates2/qquickoverlay.cpp
@@ -58,6 +58,8 @@ public:
     void destroyOverlay(QQuickPopup *popup);
     void resizeOverlay(QQuickPopup *popup);
 
+    QVector<QQuickPopup *> stackingOrderPopups() const;
+
     QQmlComponent *modal;
     QQmlComponent *modeless;
     QVector<QQuickDrawer *> drawers;
@@ -148,6 +150,22 @@ void QQuickOverlayPrivate::resizeOverlay(QQuickPopup *popup)
     }
 }
 
+QVector<QQuickPopup *> QQuickOverlayPrivate::stackingOrderPopups() const
+{
+    const QList<QQuickItem *> children = paintOrderChildItems();
+
+    QVector<QQuickPopup *> popups;
+    popups.reserve(children.count());
+
+    for (auto it = children.crbegin(), end = children.crend(); it != end; ++it) {
+        QQuickPopup *popup = qobject_cast<QQuickPopup *>((*it)->parent());
+        if (popup)
+            popups += popup;
+    }
+
+    return popups;
+}
+
 QQuickOverlayPrivate::QQuickOverlayPrivate() :
     modal(nullptr),
     modeless(nullptr),
@@ -254,22 +272,25 @@ bool QQuickOverlay::event(QEvent *event)
 {
     Q_D(QQuickOverlay);
     switch (event->type()) {
-    case QEvent::MouseButtonPress:
+    case QEvent::MouseButtonPress: {
         emit pressed();
-        for (auto it = d->popups.crbegin(), end = d->popups.crend(); it != end; ++it) {
-            if ((*it)->overlayEvent(this, event)) {
-                d->mouseGrabberPopup = *it;
+        const auto popups = d->stackingOrderPopups();
+        for (QQuickPopup *popup : popups) {
+            if (popup->overlayEvent(this, event)) {
+                d->mouseGrabberPopup = popup;
                 return true;
             }
         }
         break;
+    }
     case QEvent::MouseMove:
         if (d->mouseGrabberPopup) {
             if (d->mouseGrabberPopup->overlayEvent(this, event))
                 return true;
         } else {
-            for (auto it = d->popups.crbegin(), end = d->popups.crend(); it != end; ++it) {
-                if ((*it)->overlayEvent(this, event))
+            const auto popups = d->stackingOrderPopups();
+            for (QQuickPopup *popup : popups) {
+                if (popup->overlayEvent(this, event))
                     return true;
             }
         }
@@ -282,8 +303,9 @@ bool QQuickOverlay::event(QEvent *event)
             if (grabber->overlayEvent(this, event))
                 return true;
         } else {
-            for (auto it = d->popups.crbegin(), end = d->popups.crend(); it != end; ++it) {
-                if ((*it)->overlayEvent(this, event))
+            const auto popups = d->stackingOrderPopups();
+            for (QQuickPopup *popup : popups) {
+                if (popup->overlayEvent(this, event))
                     return true;
             }
         }
@@ -306,14 +328,12 @@ bool QQuickOverlay::childMouseEventFilter(QQuickItem *item, QEvent *event)
     while (item->parentItem() != this)
         item = item->parentItem();
 
-    const QList<QQuickItem *> sortedChildren = d->paintOrderChildItems();
-    for (auto it = sortedChildren.rbegin(), end = sortedChildren.rend(); it != end; ++it) {
-        QQuickItem *popupItem = *it;
-        if (popupItem == item)
+    const auto popups = d->stackingOrderPopups();
+    for (QQuickPopup *popup : popups) {
+        if (popup->popupItem() == item)
             break;
 
-        QQuickPopup *popup = qobject_cast<QQuickPopup *>(popupItem->parent());
-        if (popup && popup->overlayEvent(item, event))
+        if (popup->overlayEvent(item, event))
             return true;
     }
 
diff --git a/tests/auto/popup/data/applicationwindow.qml b/tests/auto/popup/data/applicationwindow.qml
index 36430609..2e868192 100644
--- a/tests/auto/popup/data/applicationwindow.qml
+++ b/tests/auto/popup/data/applicationwindow.qml
@@ -46,6 +46,7 @@ ApplicationWindow {
     height: 400
 
     property alias popup: popup
+    property alias popup2: popup2
     property alias button: button
 
     Button {
@@ -69,4 +70,14 @@ ApplicationWindow {
             }
         }
     }
+
+    Popup {
+        id: popup2
+        y: popup.y
+        z: 1
+        contentItem: Text {
+            text: "Popup2"
+            font.pixelSize: 36
+        }
+    }
 }
diff --git a/tests/auto/popup/tst_popup.cpp b/tests/auto/popup/tst_popup.cpp
index 39956301..7a8a6557 100644
--- a/tests/auto/popup/tst_popup.cpp
+++ b/tests/auto/popup/tst_popup.cpp
@@ -53,6 +53,7 @@ class tst_popup : public QQmlDataTest
 private slots:
     void visible();
     void overlay();
+    void zOrder();
     void windowChange();
     void closePolicy_data();
     void closePolicy();
@@ -154,6 +155,39 @@ void tst_popup::overlay()
     QVERIFY(!overlay->isVisible());
 }
 
+void tst_popup::zOrder()
+{
+    QQuickApplicationHelper helper(this, QStringLiteral("applicationwindow.qml"));
+
+    QQuickApplicationWindow *window = helper.window;
+    window->show();
+    window->requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(window));
+
+    QQuickPopup *popup = helper.window->property("popup").value<QQuickPopup*>();
+    QVERIFY(popup);
+    popup->setModal(true);
+
+    QQuickPopup *popup2 = helper.window->property("popup2").value<QQuickPopup*>();
+    QVERIFY(popup2);
+    popup2->setModal(true);
+
+    // show popups in reverse order. popup2 has higher z-order so it appears
+    // on top and must be closed first, even if the other popup was opened last
+    popup2->open();
+    popup->open();
+    QVERIFY(popup2->isVisible());
+    QVERIFY(popup->isVisible());
+
+    QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(1, 1));
+    QVERIFY(!popup2->isVisible());
+    QVERIFY(popup->isVisible());
+
+    QTest::mouseClick(window, Qt::LeftButton, Qt::NoModifier, QPoint(1, 1));
+    QVERIFY(!popup2->isVisible());
+    QVERIFY(!popup->isVisible());
+}
+
 void tst_popup::windowChange()
 {
     QQuickPopup popup;