QMenu: don't crash when nested tear-off menus are closed

QMenu's causedStack maintains a list of menus on the way to the menu,
and might contain nullptr if one of the entries was a tear-off menu
that got closed (and thus destroyed, due to DeleteOnClose).

If the entry we get from the stack is nullptr, fall back to the passed-
in parent widget pointer, and test for nullptr before accessing.

Add a test case that crashes without the fix.

Fixes: QTBUG-112217
Change-Id: I958182db47c3cc8733e1780f7efef43881ffae11
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
(cherry picked from commit f0049873d2ce0742a2df7ce265db70ca8baa8442)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Volker Hilsheimer 2023-06-06 13:17:37 +02:00 committed by Qt Cherry-pick Bot
parent e20ebd5a26
commit c34af8f514
2 changed files with 82 additions and 1 deletions

View File

@ -93,7 +93,9 @@ public:
Q_D(QTornOffMenu);
// make the torn-off menu a sibling of p (instead of a child)
QWidget *parentWidget = d->causedStack.isEmpty() ? p : d->causedStack.constLast();
if (parentWidget->parentWidget())
if (!parentWidget && p)
parentWidget = p;
if (parentWidget && parentWidget->parentWidget())
parentWidget = parentWidget->parentWidget();
setParent(parentWidget, Qt::Window | Qt::Tool);
setAttribute(Qt::WA_DeleteOnClose, true);

View File

@ -28,6 +28,7 @@
#include <qpa/qplatformintegration.h>
#include <QtWidgets/private/qapplication_p.h>
#include <QtWidgets/private/qmenu_p.h>
using namespace QTestPrivate;
@ -114,6 +115,8 @@ private slots:
void screenOrientationChangedCloseMenu();
void deleteWhenTriggered();
void nestedTearOffDetached();
protected slots:
void onActivated(QAction*);
void onHighlighted(QAction*);
@ -2035,5 +2038,81 @@ void tst_QMenu::deleteWhenTriggered()
QTRY_VERIFY(!menu);
}
/*
QMenu uses the caused-stack to create the parent/child relationship
for tear-off menus. Since QTornOffMenu set the DeleteOnClose flag, closing a
tear-off in the parent chain will result in a null-pointer in the caused-stack.
Verify that we don't crash when traversing the chain, as reported in QTBUG-112217.
The test has to open the submenus by hovering of the menu action, otherwise
the caused-stack remains empty and the issue doesn't reproduce. Due to QMenu's
timing and "sloppiness", we need to move the mouse within the action, with some
waiting and event processing in between to trigger the opening of the submenu.
If this fails we skip, as we then can't test what we are trying to test.
*/
void tst_QMenu::nestedTearOffDetached()
{
// Since QTornOffMenu is not declared in qmenuprivate.h we can't access the
// object even through QMenuPrivate. So use an event filter to watch out for
// a QTornOffMenu showing.
class TearOffWatcher : public QObject
{
public:
QMenu *tornOffMenu = nullptr;
protected:
bool eventFilter(QObject *receiver, QEvent *event) {
if (event->type() == QEvent::Show && receiver->inherits("QTornOffMenu"))
tornOffMenu = qobject_cast<QMenu *>(receiver);
return QObject::eventFilter(receiver, event);
}
} watcher;
qApp->installEventFilter(&watcher);
QWidget widget;
QMenu *menu = new QMenu("Context", &widget);
MenuMetrics mm(menu);
const int tearOffOffset = mm.fw + mm.vmargin + mm.tearOffHeight / 2;
QMenu *subMenu = menu->addMenu("SubMenu");
menu->setTearOffEnabled(true);
QMenu *subSubMenu = subMenu->addMenu("SubSubMenu");
subMenu->setTearOffEnabled(true);
subSubMenu->addAction("Action!");
subSubMenu->setTearOffEnabled(true);
widget.show();
QVERIFY(QTest::qWaitForWindowExposed(&widget));
// open and tear off context menu
menu->popup(widget.geometry().center());
QTest::mouseClick(menu, Qt::LeftButton, {}, QPoint(menu->width() / 2, tearOffOffset));
QMenu *menuTorn = watcher.tornOffMenu;
watcher.tornOffMenu = nullptr;
QVERIFY(menuTorn);
QVERIFY(QTest::qWaitForWindowExposed(menuTorn));
// open second menu and tear-off
QTest::mouseMove(menuTorn, menuTorn->actionGeometry(subMenu->menuAction()).topLeft());
QTest::qWait(100);
QTest::mouseMove(menuTorn, menuTorn->actionGeometry(subMenu->menuAction()).center());
if (!QTest::qWaitFor([subMenu]{ return subMenu->isVisible(); }))
QSKIP("Menu failed to show, skipping test");
QTest::mouseClick(subMenu, Qt::LeftButton, {}, QPoint(subMenu->width() / 2, tearOffOffset));
menuTorn = watcher.tornOffMenu;
QVERIFY(menuTorn);
QVERIFY(QTest::qWaitForWindowExposed(menuTorn));
// close the top level tear off
menu->hideTearOffMenu();
// open third menu and tear-off
QTest::mouseMove(menuTorn, menuTorn->actionGeometry(subSubMenu->menuAction()).topLeft());
QTest::qWait(100);
QTest::mouseMove(menuTorn, menuTorn->actionGeometry(subSubMenu->menuAction()).center());
QTRY_VERIFY(subSubMenu->isVisible());
QTest::mouseClick(subSubMenu, Qt::LeftButton, {}, QPoint(subSubMenu->width() / 2, tearOffOffset));
}
QTEST_MAIN(tst_QMenu)
#include "tst_qmenu.moc"