MenuBar top level menus are not closed on click after being opened

This fix revert partially what was done for resolving QTBUG-40391.
Infact the change QTBUG-40391 caused the propagation of the events
that caused the dismiss of a popupMenu. This in turn caused this bug
where the click that should close menu is forwarded instead of being
swallowed. However for supporting the behavior of context menu
outside menubar we added some cases where the developer expect
to receive the event that dismissed the popup. The use case is to
reopen a dismissed popup on right click (see windows right click
behavior on right clicks)

Task-number: QTBUG-45315
Change-Id: If5a181b65ed9d879521ac20de577510908125169
Reviewed-by: Filippo Cucchetto <filippocucchetto@gmail.com>
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@theqtcompany.com>

6 files changed, 130 insertions, 20 deletions

diff --git a/src/controls/qquickmenupopupwindow.cpp b/src/controls/qquickmenupopupwindow.cpp
index f5719908..c43ea233 100644
--- a/src/controls/qquickmenupopupwindow.cpp
+++ b/src/controls/qquickmenupopupwindow.cpp
@@ -42,6 +42,7 @@
 #include <QtGui/QScreen>
 #include <QtQuick/QQuickRenderControl>
 #include "qquickmenu_p.h"
+#include "qquickmenubar_p.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -151,4 +152,40 @@ QQuickMenu *QQuickMenuPopupWindow::menu() const
     return m_menu;
 }
 
+QQuickMenuBar *QQuickMenuPopupWindow::menuBar() const
+{
+    QObject *pi = menu()->parentMenuOrMenuBar();
+    while (pi) {
+        if (QQuickMenuBar *menuBar = qobject_cast<QQuickMenuBar*>(pi))
+            return menuBar;
+        else if (QQuickMenu *menu = qobject_cast<QQuickMenu*>(pi))
+            pi = menu->parentMenuOrMenuBar();
+        else
+            return 0;
+    }
+    return 0;
+}
+
+bool QQuickMenuPopupWindow::shouldForwardEventAfterDismiss(QMouseEvent *e) const
+{
+    // The events that dismissed a popup child of a menu contained in the menubar
+    // are never forwarded
+    if (QQuickMenuBar *mb = menuBar()) {
+        QPoint parentPos = transientParent()->mapFromGlobal(mapToGlobal(e->pos()));
+        if (!mb->isNative() && mb->contentItem()->contains(parentPos))
+            return false;
+    }
+
+#ifdef Q_OS_OSX
+    if (e->button() == Qt::RightButton)
+        return true;
+#endif
+
+#ifdef Q_OS_WIN
+    return true;
+#endif
+
+    return false;
+}
+
 QT_END_NAMESPACE
diff --git a/src/controls/qquickmenupopupwindow_p.h b/src/controls/qquickmenupopupwindow_p.h
index 517ba6a0..4f970c2a 100644
--- a/src/controls/qquickmenupopupwindow_p.h
+++ b/src/controls/qquickmenupopupwindow_p.h
@@ -42,6 +42,7 @@
 QT_BEGIN_NAMESPACE
 
 class QQuickMenu;
+class QQuickMenuBar;
 
 class QQuickMenuPopupWindow : public QQuickPopupWindow
 {
@@ -63,6 +64,7 @@ protected Q_SLOTS:
 
 protected:
     void exposeEvent(QExposeEvent *);
+    bool shouldForwardEventAfterDismiss(QMouseEvent *) const;
 
 private:
     QQuickItem *m_itemAt;
@@ -70,6 +72,9 @@ private:
     QPointF m_initialPos;
     QQuickWindow *m_logicalParentWindow;
     QQuickMenu *m_menu;
+
+private:
+    QQuickMenuBar *menuBar() const;
 };
 
 QT_END_NAMESPACE
diff --git a/src/controls/qquickpopupwindow.cpp b/src/controls/qquickpopupwindow.cpp
index ec0ab07c..8db73701 100644
--- a/src/controls/qquickpopupwindow.cpp
+++ b/src/controls/qquickpopupwindow.cpp
@@ -178,19 +178,28 @@ void QQuickPopupWindow::mouseReleaseEvent(QMouseEvent *e)
 
 void QQuickPopupWindow::forwardEventToTransientParent(QMouseEvent *e)
 {
+    bool forwardEvent = true;
+
     if (!qobject_cast<QQuickPopupWindow*>(transientParent())
         && ((m_mouseMoved && e->type() == QEvent::MouseButtonRelease)
             || e->type() == QEvent::MouseButtonPress)) {
         // Clicked outside any popup
         dismissPopup();
+        forwardEvent = shouldForwardEventAfterDismiss(e);
     }
-    if (transientParent()) {
+
+    if (forwardEvent && transientParent()) {
         QPoint parentPos = transientParent()->mapFromGlobal(mapToGlobal(e->pos()));
         QMouseEvent pe = QMouseEvent(e->type(), parentPos, e->button(), e->buttons(), e->modifiers());
         QGuiApplication::sendEvent(transientParent(), &pe);
     }
 }
 
+bool QQuickPopupWindow::shouldForwardEventAfterDismiss(QMouseEvent*) const
+{
+    return false;
+}
+
 void QQuickPopupWindow::exposeEvent(QExposeEvent *e)
 {
     if (isExposed() && m_needsActivatedEvent) {
diff --git a/src/controls/qquickpopupwindow_p.h b/src/controls/qquickpopupwindow_p.h
index 846bc47b..294a84f5 100644
--- a/src/controls/qquickpopupwindow_p.h
+++ b/src/controls/qquickpopupwindow_p.h
@@ -73,6 +73,7 @@ protected:
     void mouseMoveEvent(QMouseEvent *);
     void exposeEvent(QExposeEvent *);
     void hideEvent(QHideEvent *);
+    virtual bool shouldForwardEventAfterDismiss(QMouseEvent *) const;
 
 protected Q_SLOTS:
     void updateSize();
diff --git a/tests/auto/menubar/menubar.pro b/tests/auto/menubar/menubar.pro
index 41a9eaac..663100da 100644
--- a/tests/auto/menubar/menubar.pro
+++ b/tests/auto/menubar/menubar.pro
@@ -4,13 +4,22 @@ TARGET = tst_menubar
 HEADERS += \
     $$PWD/../../../src/controls/qquickpopupwindow_p.h \
     $$PWD/../../../src/controls/qquickmenupopupwindow_p.h \
-    $$PWD/../../../src/controls/qquickmenubar_p.h
+    $$PWD/../../../src/controls/qquickmenubar_p.h \
+    $$PWD/../../../src/controls/qquickmenu_p.h \
+    $$PWD/../../../src/controls/qquickmenuitem_p.h \
+    $$PWD/../../../src/controls/qquickaction_p.h \
+    $$PWD/../../../src/controls/qquickexclusivegroup_p.h \
+    $$PWD/../../../src/controls/qquickmenuitemcontainer_p.h
 
 SOURCES += \
     tst_menubar.cpp \
     $$PWD/../../../src/controls/qquickpopupwindow.cpp \
     $$PWD/../../../src/controls/qquickmenupopupwindow.cpp \
-    $$PWD/../../../src/controls/qquickmenubar.cpp
+    $$PWD/../../../src/controls/qquickmenubar.cpp \
+    $$PWD/../../../src/controls/qquickmenu.cpp \
+    $$PWD/../../../src/controls/qquickmenuitem.cpp \
+    $$PWD/../../../src/controls/qquickaction.cpp \
+    $$PWD/../../../src/controls/qquickexclusivegroup.cpp
 
 include (../shared/util.pri)
 
diff --git a/tests/auto/menubar/tst_menubar.cpp b/tests/auto/menubar/tst_menubar.cpp
index 1a25b07e..29ef295f 100644
--- a/tests/auto/menubar/tst_menubar.cpp
+++ b/tests/auto/menubar/tst_menubar.cpp
@@ -62,14 +62,20 @@ private slots:
     void init();
     void cleanup();
 
-    void testClickSubMenu();
     void testParentMenuForPopupsOutsideMenuBar();
     void testParentMenuForPopupsInsideMenuBar();
+    void testClickMenuBarSubMenu();
+    void testClickMenuBarRootMenu();
 
 private:
-    QQmlApplicationEngine* m_engine;
-    QQuickWindow* m_window;
-    QObject* m_menuBar;
+    void moveOnPos(QObject *window, const QPoint &point);
+    void moveOnPos(const QPoint &point);
+    void clickOnPos(QObject *window, const QPoint &point);
+    void clickOnPos(const QPoint &point);
+
+    QQmlApplicationEngine *m_engine;
+    QQuickWindow *m_window;
+    QObject *m_menuBar;
 };
 
 bool waitForRendering(QQuickWindow* window, int timeout = WAIT_TIME)
@@ -78,7 +84,7 @@ bool waitForRendering(QQuickWindow* window, int timeout = WAIT_TIME)
     return signalSpy.wait(timeout);
 }
 
-void moveOnPos(QObject* window, QPointF point)
+void tst_menubar::moveOnPos(QObject *window, const QPoint &point)
 {
     qApp->sendEvent(window, new QMouseEvent(QEvent::MouseMove,
                                             point,
@@ -87,7 +93,14 @@ void moveOnPos(QObject* window, QPointF point)
                                             0));
 }
 
-void clickOnPos(QObject* window, QPointF point)
+void tst_menubar::moveOnPos(const QPoint &point)
+{
+    QPoint global = m_window->mapToGlobal(point);
+    QWindow *focusWindow = qApp->focusWindow();
+    moveOnPos(focusWindow, focusWindow->mapFromGlobal(global));
+}
+
+void tst_menubar::clickOnPos(QObject *window, const QPoint &point)
 {
     qApp->sendEvent(window, new QMouseEvent(QEvent::MouseButtonPress,
                                             point,
@@ -102,6 +115,13 @@ void clickOnPos(QObject* window, QPointF point)
                                             0));
 }
 
+void tst_menubar::clickOnPos(const QPoint &point)
+{
+    QPoint global = m_window->mapToGlobal(point);
+    QWindow *focusWindow = qApp->focusWindow();
+    clickOnPos(focusWindow, focusWindow->mapFromGlobal(global));
+}
+
 void tst_menubar::init()
 {
     m_engine = new QQmlApplicationEngine();
@@ -124,15 +144,44 @@ void tst_menubar::cleanup()
     m_engine = 0;
 }
 
-void tst_menubar::testClickSubMenu()
+void tst_menubar::testClickMenuBarRootMenu()
 {
     QObject* fileMenu = m_menuBar->findChildren<QObject*>("fileMenu").first();
     QVERIFY(fileMenu);
     QCOMPARE(fileMenu->property("__popupVisible").toBool(), false);
-    clickOnPos(m_window, QPointF(5,5));
-    waitForRendering(m_window);
-    QCOMPARE(fileMenu->property("__popupVisible").toBool(), true);
+
+    // Clicking two times should open and close
+    {
+        clickOnPos(QPoint(5,5));
+        QTest::qWait(WAIT_TIME);
+        QCOMPARE(fileMenu->property("__popupVisible").toBool(), true);
+
+        clickOnPos(QPoint(5,5));
+        QTest::qWait(WAIT_TIME);;
+        QCOMPARE(fileMenu->property("__popupVisible").toBool(), false);
+    }
+
+    // Clicking outside should close the menu as well
+    {
+        clickOnPos(QPoint(5,5));
+        QTest::qWait(WAIT_TIME);
+        QCOMPARE(fileMenu->property("__popupVisible").toBool(), true);
+
+        clickOnPos(QPoint(300,300));
+        QTest::qWait(WAIT_TIME);;
+        QCOMPARE(fileMenu->property("__popupVisible").toBool(), false);
+    }
+}
+
+void tst_menubar::testClickMenuBarSubMenu()
+{
+    QObject *fileMenu = m_menuBar->findChildren<QObject*>("fileMenu").first();
+    QVERIFY(fileMenu);
+    QCOMPARE(fileMenu->property("__popupVisible").toBool(), false);
+
+    clickOnPos(QPoint(5,5));
     QTest::qWait(WAIT_TIME);
+    QCOMPARE(fileMenu->property("__popupVisible").toBool(), true);
 
     QQuickItem* fileMenuContentItem = fileMenu->property("__contentItem").value<QQuickItem*>();
     QVERIFY(fileMenuContentItem);
@@ -146,17 +195,17 @@ void tst_menubar::testClickSubMenu()
     QVERIFY(actionsSubMenuContentItem);
 
     // Click on a submenu should open it
-    clickOnPos(fileMenuContentItem->window(), QPointF(5,5));
+    clickOnPos(QPoint(5,25));
     QTest::qWait(WAIT_TIME);
     QCOMPARE(actionsSubMenu->property("__popupVisible").toBool(), true);
 
     // Click on a submenu should not close the popup
-    clickOnPos(actionsSubMenuContentItem->window(), QPointF(-5,0));
+    clickOnPos(QPoint(5,25));
     QTest::qWait(WAIT_TIME);
     QCOMPARE(actionsSubMenu->property("__popupVisible").toBool(), true);
 
     // Click outside should close the popup
-    clickOnPos(actionsSubMenuContentItem->window(), QPointF(100,100));
+    clickOnPos(QPoint(100,100));
     QTest::qWait(WAIT_TIME);
     QCOMPARE(actionsSubMenu->property("__popupVisible").toBool(), false);
 }
@@ -165,8 +214,8 @@ void tst_menubar::testParentMenuForPopupsOutsideMenuBar()
 {
     waitForRendering(m_window);
     QCOMPARE(qApp->focusWindow() == m_window, true);
-    moveOnPos(m_window, QPointF(50,50));
-    clickOnPos(m_window, QPointF(50,50));
+    moveOnPos(QPoint(50,50));
+    clickOnPos(QPoint(50,50));
     QTest::qWait(500);
     QCOMPARE(qApp->focusWindow() == m_window, false);
 
@@ -183,8 +232,8 @@ void tst_menubar::testParentMenuForPopupsInsideMenuBar()
 {
     waitForRendering(m_window);
     QCOMPARE(qApp->focusWindow() == m_window, true);
-    moveOnPos(m_window, QPointF(5,5));
-    clickOnPos(m_window, QPointF(5,5));
+    moveOnPos(QPoint(5,5));
+    clickOnPos(QPoint(5,5));
     QTest::qWait(500);
     QCOMPARE(qApp->focusWindow() == m_window, false);