diff --git a/src/quicktemplates/qquickmenu.cpp b/src/quicktemplates/qquickmenu.cpp index 5df434ee02..8686cff954 100644 --- a/src/quicktemplates/qquickmenu.cpp +++ b/src/quicktemplates/qquickmenu.cpp @@ -907,10 +907,16 @@ void QQuickMenuPrivate::itemSiblingOrderChanged(QQuickItem *) void QQuickMenuPrivate::itemDestroyed(QQuickItem *item) { - QQuickPopupPrivate::itemDestroyed(item); - int index = contentModel->indexOf(item, nullptr); - if (index != -1) - removeItem(index, item); + if (item == contentItem) { + resetContentItem(); + } else { + QQuickPopupPrivate::itemDestroyed(item); + if (contentModel) { + int index = contentModel->indexOf(item, nullptr); + if (index != -1) + removeItem(index, item); + } + } } void QQuickMenuPrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometryChange, const QRectF &) @@ -1343,6 +1349,20 @@ void QQuickMenuPrivate::contentData_clear(QQmlListProperty *prop) QQuickMenuPrivate::get(q)->contentData.clear(); } +void QQuickMenuPrivate::resetContentItem() +{ + if (contentItem) { + QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Children); + QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Destroyed); + QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Geometry); + + const auto children = contentItem->childItems(); + for (QQuickItem *child : std::as_const(children)) + QQuickItemPrivate::get(child)->removeItemChangeListener(this, QQuickItemPrivate::SiblingOrder); + contentItem = nullptr; + } +} + QQuickMenu::QQuickMenu(QObject *parent) : QQuickPopup(*(new QQuickMenuPrivate), parent) { @@ -1359,6 +1379,15 @@ QQuickMenu::~QQuickMenu() << "item count:" << d->contentModel->count() << "native item count:" << d->nativeItems.count(); + // It would be better to reset the sub-menu within the menu-item during its destruction + // as there can be a chance that the parent menu use invalid reference leading to + // application crash (as mentioned in the bug report QTBUG-137160) + if (auto *menuItem = qobject_cast(d->parentItem)) { + if (menuItem->subMenu() == this) { + auto *menuItemPriv = QQuickMenuItemPrivate::get(menuItem); + menuItemPriv->setSubMenu(nullptr); + } + } // We have to remove items to ensure that our change listeners on the item // are removed. It's too late to do this in ~QQuickMenuPrivate, as // contentModel has already been destroyed before that is called. @@ -1367,14 +1396,7 @@ QQuickMenu::~QQuickMenu() while (d->contentModel->count() > 0) d->removeItem(0, d->itemAt(0), QQuickMenuPrivate::DestructionPolicy::Destroy); - if (d->contentItem) { - QQuickItemPrivate::get(d->contentItem)->removeItemChangeListener(d, QQuickItemPrivate::Children); - QQuickItemPrivate::get(d->contentItem)->removeItemChangeListener(d, QQuickItemPrivate::Geometry); - - const auto children = d->contentItem->childItems(); - for (QQuickItem *child : std::as_const(children)) - QQuickItemPrivate::get(child)->removeItemChangeListener(d, QQuickItemPrivate::SiblingOrder); - } + d->resetContentItem(); } /*! @@ -2173,10 +2195,12 @@ void QQuickMenu::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem) if (oldItem) { QQuickItemPrivate::get(oldItem)->removeItemChangeListener(d, QQuickItemPrivate::Children); + QQuickItemPrivate::get(newItem)->removeItemChangeListener(d, QQuickItemPrivate::Destroyed); QQuickItemPrivate::get(oldItem)->removeItemChangeListener(d, QQuickItemPrivate::Geometry); } if (newItem) { QQuickItemPrivate::get(newItem)->addItemChangeListener(d, QQuickItemPrivate::Children); + QQuickItemPrivate::get(newItem)->addItemChangeListener(d, QQuickItemPrivate::Destroyed); QQuickItemPrivate::get(newItem)->updateOrAddGeometryChangeListener(d, QQuickGeometryChange::Width); } diff --git a/src/quicktemplates/qquickmenu_p_p.h b/src/quicktemplates/qquickmenu_p_p.h index 42ad2c77c4..ec8d5f2e1c 100644 --- a/src/quicktemplates/qquickmenu_p_p.h +++ b/src/quicktemplates/qquickmenu_p_p.h @@ -131,6 +131,8 @@ public: QPalette defaultPalette() const override; virtual QQuickPopup::PopupType resolvedPopupType() const override; + void resetContentItem(); + bool cascade = false; bool triedToCreateNativeMenu = false; int hoverTimer = 0; @@ -139,9 +141,9 @@ public: qreal textPadding = 0; QPointer parentMenu; QPointer currentItem; - QQuickItem *contentItem = nullptr; // TODO: cleanup + QPointer contentItem; QList contentData; - QQmlObjectModel *contentModel; + QPointer contentModel; QQmlComponent *delegate = nullptr; QString title; QQuickIcon icon; diff --git a/src/quicktemplates/qquickmenuitem_p_p.h b/src/quicktemplates/qquickmenuitem_p_p.h index 3ef4981570..bc1e585985 100644 --- a/src/quicktemplates/qquickmenuitem_p_p.h +++ b/src/quicktemplates/qquickmenuitem_p_p.h @@ -47,7 +47,7 @@ public: bool highlighted = false; QQuickDeferredPointer arrow; QQuickMenu *menu = nullptr; - QQuickMenu *subMenu = nullptr; + QPointer subMenu; qreal implicitTextPadding; }; diff --git a/tests/auto/quickcontrols/qquickmenu/data/loadMenuAsynchronously.qml b/tests/auto/quickcontrols/qquickmenu/data/loadMenuAsynchronously.qml new file mode 100644 index 0000000000..3e73044a20 --- /dev/null +++ b/tests/auto/quickcontrols/qquickmenu/data/loadMenuAsynchronously.qml @@ -0,0 +1,62 @@ +import QtQuick +import QtQuick.Controls + +Item { + width:100 + height:100 + + property alias loader: menuLoader + property bool activateLoader: false + + onActivateLoaderChanged: { + if (activateLoader) { + menuTimer.start() + menuLoader.active = true + } + } + + Timer { + id: menuTimer + interval: 20 + onTriggered: menuLoader.active = false + } + + Loader { + id: menuLoader + active: false + asynchronous: true + sourceComponent: Menu { + contentItem: ListView {} + Menu {} + MenuSeparator {} + Action {} + Menu { + Action {} + Action {} + Menu {} + } + MenuItem {} + Item { + Menu {} + } + Menu { + MenuItem {} + Action {} + MenuItem {} + } + // Dummy menu items + Menu {} + Menu {} + MenuSeparator {} + Menu {} + Menu {} + MenuSeparator {} + Menu {} + Menu {} + MenuSeparator {} + Menu {} + Menu {} + } + } +} + diff --git a/tests/auto/quickcontrols/qquickmenu/tst_qquickmenu.cpp b/tests/auto/quickcontrols/qquickmenu/tst_qquickmenu.cpp index 0387df813d..aefa8c1e0d 100644 --- a/tests/auto/quickcontrols/qquickmenu/tst_qquickmenu.cpp +++ b/tests/auto/quickcontrols/qquickmenu/tst_qquickmenu.cpp @@ -19,8 +19,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -127,6 +129,7 @@ private slots: void shortcutInNestedSubMenuAction(); void animationOnHeight(); void dontDeleteDelegates(); + void loadMenuAsynchronously(); private: bool nativeMenuSupported = false; @@ -3562,6 +3565,40 @@ void tst_QQuickMenu::dontDeleteDelegates() QVERIFY(delegateComponent2); } +void tst_QQuickMenu::loadMenuAsynchronously() +{ + QQuickView window(testFileUrl("loadMenuAsynchronously.qml")); + QCOMPARE(window.status(), QQuickView::Ready); + window.show(); + window.requestActivate(); + QVERIFY(QTest::qWaitForWindowActive(&window)); + + auto *rootItem = window.rootObject(); + QVERIFY(rootItem); + + auto *loader = rootItem->property("loader").value(); + QVERIFY(loader); + QCOMPARE(loader->active(), false); + QCOMPARE(loader->asynchronous(), true); + + auto activateLoader = rootItem->property("activateLoader").value(); + QCOMPARE(activateLoader, false); + QVERIFY(rootItem->setProperty("activateLoader", true)); + activateLoader = rootItem->property("activateLoader").value(); + QCOMPARE(activateLoader, true); + + QTRY_COMPARE(loader->active(), false); + + rootItem->setProperty("activateLoader", false); + activateLoader = rootItem->property("activateLoader").value(); + QCOMPARE(activateLoader, false); + QVERIFY(rootItem->setProperty("activateLoader", true)); + activateLoader = rootItem->property("activateLoader").value(); + QCOMPARE(activateLoader, true); + + QTRY_COMPARE(loader->active(), false); +} + QTEST_QUICKCONTROLS_MAIN(tst_QQuickMenu) #include "tst_qquickmenu.moc"