From d923dd467c1aeb3e195a09949b04862084002f88 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Thu, 16 Aug 2018 10:36:19 +0200 Subject: MenuBar: ensure the correct delegates are used when created via Component Don't add items until we're complete, as the delegate could change in the meantime. Instead, add them to contentData and create them when we're complete. A similar fix was already done for Menu in d5cb26bc. Task-number: QTBUG-67559 Change-Id: Idb43b7a69fcf1c1ad6396c73a3c090b92e460ab8 Reviewed-by: Qt CI Bot Reviewed-by: Richard Moe Gustavsen --- src/quicktemplates2/qquickcontainer.cpp | 25 ++++--- src/quicktemplates2/qquickcontainer_p_p.h | 2 + src/quicktemplates2/qquickmenubar.cpp | 35 +++++++++- src/quicktemplates2/qquickmenubar_p_p.h | 2 + .../data/delegateFromSeparateComponent.qml | 79 ++++++++++++++++++++++ tests/auto/qquickmenubar/tst_qquickmenubar.cpp | 27 ++++++++ 6 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 tests/auto/qquickmenubar/data/delegateFromSeparateComponent.qml diff --git a/src/quicktemplates2/qquickcontainer.cpp b/src/quicktemplates2/qquickcontainer.cpp index 3dfefdc0..b3e7454b 100644 --- a/src/quicktemplates2/qquickcontainer.cpp +++ b/src/quicktemplates2/qquickcontainer.cpp @@ -334,6 +334,21 @@ void QQuickContainerPrivate::reorderItems() } } +// Helper function needed for derived classes such as QQuickMenuBarPrivate. +void QQuickContainerPrivate::addObject(QObject *obj) +{ + Q_Q(QQuickContainer); + QQuickItem *item = qobject_cast(obj); + if (item) { + if (QQuickItemPrivate::get(item)->isTransparentForPositioner()) + item->setParentItem(effectiveContentItem(contentItem)); + else if (contentModel->indexOf(item, nullptr) == -1) + q->addItem(item); + } else { + contentData.append(obj); + } +} + void QQuickContainerPrivate::_q_currentIndexChanged() { Q_Q(QQuickContainer); @@ -375,15 +390,7 @@ void QQuickContainerPrivate::contentData_append(QQmlListProperty *prop, { QQuickContainer *q = static_cast(prop->object); QQuickContainerPrivate *p = QQuickContainerPrivate::get(q); - QQuickItem *item = qobject_cast(obj); - if (item) { - if (QQuickItemPrivate::get(item)->isTransparentForPositioner()) - item->setParentItem(effectiveContentItem(p->contentItem)); - else if (p->contentModel->indexOf(item, nullptr) == -1) - q->addItem(item); - } else { - p->contentData.append(obj); - } + p->addObject(obj); } int QQuickContainerPrivate::contentData_count(QQmlListProperty *prop) diff --git a/src/quicktemplates2/qquickcontainer_p_p.h b/src/quicktemplates2/qquickcontainer_p_p.h index 4e53aa8f..09a89b90 100644 --- a/src/quicktemplates2/qquickcontainer_p_p.h +++ b/src/quicktemplates2/qquickcontainer_p_p.h @@ -74,6 +74,8 @@ public: void removeItem(int index, QQuickItem *item); void reorderItems(); + void addObject(QObject *obj); + void _q_currentIndexChanged(); void itemChildAdded(QQuickItem *item, QQuickItem *child) override; diff --git a/src/quicktemplates2/qquickmenubar.cpp b/src/quicktemplates2/qquickmenubar.cpp index efb83a17..876cc471 100644 --- a/src/quicktemplates2/qquickmenubar.cpp +++ b/src/quicktemplates2/qquickmenubar.cpp @@ -76,6 +76,25 @@ QT_BEGIN_NAMESPACE {Focus Management in Qt Quick Controls 2} */ +void QQuickMenuBarPrivate::createItems() +{ + // removeItem() will remove stuff from contentData, so we have to make a copy of it. + const auto originalContentData = contentData; + // Sanity check that there aren't any items we don't know about. + Q_ASSERT(contentModel->count() == 0); + + for (QObject *object : originalContentData) { + if (QQuickMenu *menu = qobject_cast(object)) { + // It's a QQuickMenu; create a QQuickMenuBarItem for it. + QQuickItem *menuItem = createItem(menu); + addObject(menuItem); + } else if (qobject_cast(object)) { + addObject(object); + } + // If it's neither, skip it because we don't care about it. + } +} + QQuickItem *QQuickMenuBarPrivate::beginCreateItem() { Q_Q(QQuickMenuBar); @@ -259,9 +278,18 @@ void QQuickMenuBarPrivate::itemGeometryChanged(QQuickItem *, QQuickGeometryChang void QQuickMenuBarPrivate::contentData_append(QQmlListProperty *prop, QObject *obj) { QQuickMenuBar *menuBar = static_cast(prop->object); - if (QQuickMenu *menu = qobject_cast(obj)) - obj = QQuickMenuBarPrivate::get(menuBar)->createItem(menu); - QQuickContainerPrivate::contentData_append(prop, obj); + QQuickMenuBarPrivate *menuBarPrivate = QQuickMenuBarPrivate::get(menuBar); + if (!menuBarPrivate->componentComplete) { + // Don't add items until we're complete, as the delegate could change in the meantime. + // We'll add it to contentData and create it when we're complete. + menuBarPrivate->contentData.append(obj); + return; + } + + if (QQuickMenu *menu = qobject_cast(obj)) { + QQuickItem *menuItem = menuBarPrivate->createItem(menu); + menuBarPrivate->addObject(menuItem); + } } void QQuickMenuBarPrivate::menus_append(QQmlListProperty *prop, QQuickMenu *obj) @@ -521,6 +549,7 @@ void QQuickMenuBar::componentComplete() { Q_D(QQuickMenuBar); QQuickContainer::componentComplete(); + d->createItems(); d->updateContentSize(); } diff --git a/src/quicktemplates2/qquickmenubar_p_p.h b/src/quicktemplates2/qquickmenubar_p_p.h index b6fdc9eb..24824292 100644 --- a/src/quicktemplates2/qquickmenubar_p_p.h +++ b/src/quicktemplates2/qquickmenubar_p_p.h @@ -66,6 +66,8 @@ public: return menuBar->d_func(); } + void createItems(); + QQuickItem *beginCreateItem(); void completeCreateItem(); diff --git a/tests/auto/qquickmenubar/data/delegateFromSeparateComponent.qml b/tests/auto/qquickmenubar/data/delegateFromSeparateComponent.qml new file mode 100644 index 00000000..9d58e8c6 --- /dev/null +++ b/tests/auto/qquickmenubar/data/delegateFromSeparateComponent.qml @@ -0,0 +1,79 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:BSD$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** BSD License Usage +** Alternatively, you may use this file under the terms of the BSD license +** as follows: +** +** "Redistribution and use in source and binary forms, with or without +** modification, are permitted provided that the following conditions are +** met: +** * Redistributions of source code must retain the above copyright +** notice, this list of conditions and the following disclaimer. +** * Redistributions in binary form must reproduce the above copyright +** notice, this list of conditions and the following disclaimer in +** the documentation and/or other materials provided with the +** distribution. +** * Neither the name of The Qt Company Ltd nor the names of its +** contributors may be used to endorse or promote products derived +** from this software without specific prior written permission. +** +** +** THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +** "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +** LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +** A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +** OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +** SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +** LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +** OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE." +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +import QtQuick 2.11 +import QtQuick.Controls 2.4 + +ApplicationWindow { + width: 800 + height: 800 + + Component { + id: menuBarItemComponent + + MenuBarItem { + contentItem: Text { + text: parent.text + color: "blue" + } + background: Rectangle { + color: "#00ff00" + } + } + } + + menuBar: MenuBar { + delegate: menuBarItemComponent + + Menu { + title: "Menu" + } + } +} diff --git a/tests/auto/qquickmenubar/tst_qquickmenubar.cpp b/tests/auto/qquickmenubar/tst_qquickmenubar.cpp index 754e915b..1d5844ee 100644 --- a/tests/auto/qquickmenubar/tst_qquickmenubar.cpp +++ b/tests/auto/qquickmenubar/tst_qquickmenubar.cpp @@ -62,6 +62,7 @@ private slots: void keys(); void mnemonics(); void addRemove(); + void delegateFromSeparateComponent(); }; void tst_qquickmenubar::delegate() @@ -570,6 +571,32 @@ void tst_qquickmenubar::addRemove() QVERIFY(menuBarItem1.isNull()); } +// QTBUG-67559 +// Test that Menus declared as children of a MenuBar have the +// correct delegate when it is declared outside of the MenuBar as a Component. +void tst_qquickmenubar::delegateFromSeparateComponent() +{ + QQuickApplicationHelper helper(this, QLatin1String("delegateFromSeparateComponent.qml")); + QQuickWindow *window = helper.window; + window->show(); + QVERIFY(QTest::qWaitForWindowActive(window)); + + const QColor green = QColor::fromRgb(0x00ff00); + + QQuickMenuBar *menuBar = window->property("menuBar").value(); + QVERIFY(menuBar); + + QQuickMenu *menu = qobject_cast(menuBar->menuAt(0)); + QVERIFY(menu); + + QQuickMenuBarItem *menuBarItem = qobject_cast(menu->parentItem()); + QVERIFY(menuBarItem); + + QQuickItem *menuBarItemBg = menuBarItem->property("background").value(); + QVERIFY(menuBarItemBg); + QCOMPARE(menuBarItemBg->property("color").value(), green); +} + QTEST_QUICKCONTROLS_MAIN(tst_qquickmenubar) #include "tst_qquickmenubar.moc" -- cgit v1.2.3 From d56c193eb4ceb640611d66f22e1f26aae91cd7d1 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 26 Sep 2018 15:16:41 +0200 Subject: QQuickPopupPositioner: avoid adding duplicate item change listeners The issue is that QQuickPopupPositioner::setParentItem() is called when the delegate has been created and assigned to the Repeater, then the ancestor listeners are added, and then straight after that, the benchmark item itself is parented to benchmarkRoot, which causes QQuickPopupPositioner::itemParentChanged() to be called, which adds a single ancestor listener: the QQuickRootItem (which was just added previously as a result of QQuickPopupPositioner::setParentItem() being called). The item could be arbitrarily high up in the ancestry tree, so there's no nice (i.e. fast) way of checking for duplicates in Controls 2 itself. Instead, use the new QQuickItemPrivate::updateOrAddItemChangeListener() function which only adds the listener if it doesn't already exist. This avoids a heap-use-after-free in qmlbench when creating Menus. Task-number: QTBUG-70729 Change-Id: I0efaa10167c4c9a9c4c1b65a5c34e683c3ec5732 Fixes: QTBUG-70729 Reviewed-by: Michael Brasser Reviewed-by: Richard Moe Gustavsen --- src/quicktemplates2/qquickpopuppositioner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/quicktemplates2/qquickpopuppositioner.cpp b/src/quicktemplates2/qquickpopuppositioner.cpp index 841eac9c..4fe5135c 100644 --- a/src/quicktemplates2/qquickpopuppositioner.cpp +++ b/src/quicktemplates2/qquickpopuppositioner.cpp @@ -269,7 +269,7 @@ void QQuickPopupPositioner::addAncestorListeners(QQuickItem *item) QQuickItem *p = item; while (p) { - QQuickItemPrivate::get(p)->addItemChangeListener(this, AncestorChangeTypes); + QQuickItemPrivate::get(p)->updateOrAddItemChangeListener(this, AncestorChangeTypes); p = p->parentItem(); } } -- cgit v1.2.3