QMenu: Fix torn-off menu display crash issue

When tearing off either a non-scrollable multi-colume menu
or a scrollable menu, displaying the torn-off menu crashes.
The root cause is when the torn-off menu is created, the
tear-off menu's style, margins and other attributes are not
set to it. The patch is to ensure the torn-off menu has
the same attributes as the tear-off menu does and set the
torn-off menu with a correct menu size.

Task-number: QTBUG-24815
Change-Id: Icea45f149ea8792671af4a62e62cad6ee01a1f95
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@qt.io>

2 files changed, 81 insertions, 9 deletions

diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp
index f08f573391..995a08e310 100644
--- a/src/widgets/widgets/qmenu.cpp
+++ b/src/widgets/widgets/qmenu.cpp
@@ -84,18 +84,38 @@ class QTornOffMenu : public QMenu
     Q_OBJECT
     class QTornOffMenuPrivate : public QMenuPrivate
     {
-        Q_DECLARE_PUBLIC(QMenu)
+        Q_DECLARE_PUBLIC(QTornOffMenu)
     public:
-        QTornOffMenuPrivate(QMenu *p) : causedMenu(p) {
+        QTornOffMenuPrivate(QMenu *p) : causedMenu(p), initialized(false) {
             tornoff = 1;
             causedPopup.widget = 0;
-            causedPopup.action = ((QTornOffMenu*)p)->d_func()->causedPopup.action;
-            causedStack = ((QTornOffMenu*)p)->d_func()->calcCausedStack();
+            causedPopup.action = p->d_func()->causedPopup.action;
+            causedStack = p->d_func()->calcCausedStack();
+        }
+
+        void setMenuSize(const QSize &menuSize) {
+            Q_Q(QTornOffMenu);
+            QSize size = menuSize;
+            const QPoint p = (!initialized) ? causedMenu->pos() : q->pos();
+            QRect screen = popupGeometry(QApplication::desktop()->screenNumber(p));
+            const int desktopFrame = q->style()->pixelMetric(QStyle::PM_MenuDesktopFrameWidth, 0, q);
+            const int titleBarHeight = q->style()->pixelMetric(QStyle::PM_TitleBarHeight, 0, q);
+            if (scroll && (size.height() > screen.height() - titleBarHeight || size.width() > screen.width())) {
+                const int fw = q->style()->pixelMetric(QStyle::PM_MenuPanelWidth, 0, q);
+                const int hmargin = q->style()->pixelMetric(QStyle::PM_MenuHMargin, 0, q);
+                scroll->scrollFlags |= uint(QMenuPrivate::QMenuScroller::ScrollDown);
+                size.setWidth(qMin(actionRects.at(getLastVisibleAction()).right() + fw + hmargin + rightmargin + 1, screen.width()));
+                size.setHeight(screen.height() - desktopFrame * 2 - titleBarHeight);
+            }
+            q->setFixedSize(size);
         }
+
         QVector<QPointer<QWidget> > calcCausedStack() const Q_DECL_OVERRIDE { return causedStack; }
         QPointer<QMenu> causedMenu;
         QVector<QPointer<QWidget> > causedStack;
+        bool initialized;
     };
+
 public:
     QTornOffMenu(QMenu *p) : QMenu(*(new QTornOffMenuPrivate(p)))
     {
@@ -109,11 +129,20 @@ public:
         setAttribute(Qt::WA_X11NetWmWindowTypeMenu, true);
         setWindowTitle(p->windowTitle());
         setEnabled(p->isEnabled());
+        setStyleSheet(p->styleSheet());
+        if (style() != p->style())
+            setStyle(p->style());
+        int leftMargin, topMargin, rightMargin, bottomMargin;
+        p->getContentsMargins(&leftMargin, &topMargin, &rightMargin, &bottomMargin);
+        setContentsMargins(leftMargin, topMargin, rightMargin, bottomMargin);
+        setLayoutDirection(p->layoutDirection());
         //QObject::connect(this, SIGNAL(triggered(QAction*)), this, SLOT(onTrigger(QAction*)));
         //QObject::connect(this, SIGNAL(hovered(QAction*)), this, SLOT(onHovered(QAction*)));
         QList<QAction*> items = p->actions();
         for(int i = 0; i < items.count(); i++)
             addAction(items.at(i));
+        d->setMenuSize(sizeHint());
+        d->initialized = true;
     }
     void syncWithMenu(QMenu *menu, QActionEvent *act)
     {
@@ -127,12 +156,17 @@ public:
     }
     void actionEvent(QActionEvent *e) Q_DECL_OVERRIDE
     {
+        Q_D(QTornOffMenu);
         QMenu::actionEvent(e);
-        setFixedSize(sizeHint());
+        if (d->initialized) {
+            d->setMenuSize(sizeHint());
+        }
     }
+
 public slots:
     void onTrigger(QAction *action) { d_func()->activateAction(action, QAction::Trigger, false); }
     void onHovered(QAction *action) { d_func()->activateAction(action, QAction::Hover, false); }
+
 private:
     Q_DECLARE_PRIVATE(QTornOffMenu)
     friend class QMenuPrivate;
@@ -369,9 +403,11 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
     }
 
     max_column_width += tabWidth; //finally add in the tab width
-    const int sfcMargin = style->sizeFromContents(QStyle::CT_Menu, &opt, QApplication::globalStrut(), q).width() - QApplication::globalStrut().width();
-    const int min_column_width = q->minimumWidth() - (sfcMargin + leftmargin + rightmargin + 2 * (fw + hmargin));
-    max_column_width = qMax(min_column_width, max_column_width);
+    if (!tornoff || (tornoff && scroll)) { // exclude non-scrollable tear-off menu since the tear-off menu has a fixed size
+        const int sfcMargin = style->sizeFromContents(QStyle::CT_Menu, &opt, QApplication::globalStrut(), q).width() - QApplication::globalStrut().width();
+        const int min_column_width = q->minimumWidth() - (sfcMargin + leftmargin + rightmargin + 2 * (fw + hmargin));
+        max_column_width = qMax(min_column_width, max_column_width);
+    }
 
     //calculate position
     int x = hmargin + fw + leftmargin;
@@ -3512,7 +3548,6 @@ void QMenu::actionEvent(QActionEvent *e)
     }
 
     if (isVisible()) {
-        d->updateActionRects();
         resize(sizeHint());
         update();
     }
diff --git a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp
index 9109b2e8a3..e5e2e157c5 100644
--- a/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp
+++ b/tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp
@@ -130,6 +130,8 @@ private slots:
     void QTBUG_56917_wideSubmenuScreenNumber();
     void menuSize_Scrolling_data();
     void menuSize_Scrolling();
+    void tearOffMenuNotDisplayed();
+
 protected slots:
     void onActivated(QAction*);
     void onHighlighted(QAction*);
@@ -1556,5 +1558,40 @@ void tst_QMenu::menuSize_Scrolling()
              menu.height() - mm.fw - mm.vmargin - bottomMargin - 1);
 }
 
+void tst_QMenu::tearOffMenuNotDisplayed()
+{
+    QWidget widget;
+    QScopedPointer<QMenu> menu(new QMenu(&widget));
+    menu->setTearOffEnabled(true);
+    QVERIFY(menu->isTearOffEnabled());
+
+    menu->setStyleSheet("QMenu { menu-scrollable: 1 }");
+    for (int i = 0; i < 80; i++)
+        menu->addAction(QString::number(i));
+
+    widget.resize(300, 200);
+    centerOnScreen(&widget);
+    widget.show();
+    widget.activateWindow();
+    QVERIFY(QTest::qWaitForWindowActive(&widget));
+    menu->popup(widget.geometry().topRight() + QPoint(50, 0));
+    QVERIFY(QTest::qWaitForWindowActive(menu.data()));
+    QVERIFY(!menu->isTearOffMenuVisible());
+
+    MenuMetrics mm(menu.data());
+    const int tearOffOffset = mm.fw + mm.vmargin + mm.tearOffHeight / 2;
+
+    QTest::mouseClick(menu.data(), Qt::LeftButton, 0, QPoint(10, tearOffOffset), 10);
+    QTRY_VERIFY(menu->isTearOffMenuVisible());
+    QPointer<QMenu> torn = getTornOffMenu();
+    QVERIFY(torn);
+    QVERIFY(torn->isVisible());
+    QVERIFY(torn->minimumWidth() >=0 && torn->minimumWidth() < QWIDGETSIZE_MAX);
+
+    menu->hideTearOffMenu();
+    QVERIFY(!menu->isTearOffMenuVisible());
+    QVERIFY(!torn->isVisible());
+}
+
 QTEST_MAIN(tst_QMenu)
 #include "tst_qmenu.moc"