From 0a1d8fb26d1e89c82a0d35dd728d7e928a94d197 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Tue, 24 Sep 2024 14:44:25 +0800 Subject: [PATCH 1/1] ListView: fix countChanged not being emitted in certain cases This broke after 477c15def834bd49553c00b90f3a2006456ea931. countChanged would have normally been emitted in the call to applyModelChanges that we skipped after adding the q->size().isNull() check. We can check hasPendingChanges() to know if a count change is pending (which is also the first thing applyModelChanges checks before doing its work), and if so, emit countChanged so that any bindings that are relying on it can function as expected. Fixes: QTBUG-129165 Pick-to: 6.5 6.7 6.8 dev Change-Id: Ic045f1870b39d192f6880e23daab03fd73a16d58 Reviewed-by: Fabian Kosmale --- src/quick/items/qquickitemview.cpp | 31 +++++++++++++++++----- src/quick/items/qquickitemview_p_p.h | 2 ++ .../data/visibleBoundToCountGreaterThanZero.qml | 31 ++++++++++++++++++++++ .../quick/qquicklistview2/tst_qquicklistview2.cpp | 20 ++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 tests/auto/quick/qquicklistview2/data/visibleBoundToCountGreaterThanZero.qml diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 0a6a3e949bd..19e5b7f4029 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -10,6 +10,7 @@ QT_BEGIN_NAMESPACE Q_LOGGING_CATEGORY(lcItemViewDelegateLifecycle, "qt.quick.itemview.lifecycle") +Q_LOGGING_CATEGORY(lcCount, "qt.quick.itemview.count") // Default cacheBuffer for all views. #ifndef QML_VIEW_DEFAULTCACHEBUFFER @@ -223,7 +224,7 @@ void QQuickItemView::setModel(const QVariant &m) this, SLOT(modelUpdated(QQmlChangeSet,bool))); if (QQmlDelegateModel *dataModel = qobject_cast(d->model)) QObjectPrivate::connect(dataModel, &QQmlDelegateModel::delegateChanged, d, &QQuickItemViewPrivate::applyDelegateChange); - emit countChanged(); + d->emitCountChanged(); } emit modelChanged(); d->moveReason = QQuickItemViewPrivate::Other; @@ -255,7 +256,7 @@ void QQuickItemView::setDelegate(QQmlComponent *delegate) int oldCount = dataModel->count(); dataModel->setDelegate(delegate); if (oldCount != dataModel->count()) - emit countChanged(); + d->emitCountChanged(); } emit delegateChanged(); d->delegateValidated = false; @@ -1125,6 +1126,14 @@ void QQuickItemViewPrivate::showVisibleItems() const } } +// Simplifies debugging of count. +void QQuickItemViewPrivate::emitCountChanged() +{ + Q_Q(QQuickItemView); + qCDebug(lcCount).nospace() << "about to emit countChanged for " << q << "; count changed to " << q->count(); + emit q->countChanged(); +} + void QQuickItemViewPrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &oldGeometry) { @@ -1224,7 +1233,7 @@ void QQuickItemView::modelUpdated(const QQmlChangeSet &changeSet, bool reset) d->updateTrackedItem(); } d->moveReason = QQuickItemViewPrivate::Other; - emit countChanged(); + d->emitCountChanged(); #if QT_CONFIG(quick_viewtransitions) if (d->transitioner && d->transitioner->populateTransition) d->forceLayoutPolish(); @@ -1487,7 +1496,7 @@ void QQuickItemView::componentComplete() d->fixupPosition(); } if (d->model && d->model->count()) - emit countChanged(); + d->emitCountChanged(); } @@ -1813,7 +1822,7 @@ void QQuickItemViewPrivate::refill(qreal from, qreal to) } if (prevCount != itemCount) - emit q->countChanged(); + emitCountChanged(); } while (currentChanges.hasPendingChanges() || bufferedChanges.hasPendingChanges()); storeFirstVisibleItemPosition(); } @@ -1864,6 +1873,16 @@ void QQuickItemViewPrivate::layout() // if either dimension is negative, but apparently we support negative-sized // views (see tst_QQuickListView::resizeView). if ((!isValid() && !visibleItems.size()) || q->size().isNull()) { + if (q->size().isNull() && hasPendingChanges()) { + // count() refers to the number of items in the model, not in the view + // (which is why we don't emit for the !visibleItems.size() case). + // If there are pending model changes, emit countChanged in order to + // support the use case of QTBUG-129165, where visible is bound to count > 0 + // and the ListView is in a layout with Layout.preferredHeight bound to + // contentHeight. This ensures that a hidden ListView will become visible. + emitCountChanged(); + } + clear(); setPosition(contentStartOffset()); updateViewport(); @@ -2138,7 +2157,7 @@ bool QQuickItemViewPrivate::applyModelChanges(ChangeResult *totalInsertionResult updateSections(); if (prevItemCount != itemCount) - emit q->countChanged(); + emitCountChanged(); if (!visibleAffected && viewportChanged) updateViewport(); diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index 9cb6ea8fd5c..b5d70c6d6f4 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -229,6 +229,8 @@ public: releaseItem(item, reusableFlag); } + void emitCountChanged(); + virtual QQuickItemViewAttached *getAttachedObject(const QObject *) const { return nullptr; } QPointer model; diff --git a/tests/auto/quick/qquicklistview2/data/visibleBoundToCountGreaterThanZero.qml b/tests/auto/quick/qquicklistview2/data/visibleBoundToCountGreaterThanZero.qml new file mode 100644 index 00000000000..e75c779584e --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/visibleBoundToCountGreaterThanZero.qml @@ -0,0 +1,31 @@ +import QtQuick +import QtQuick.Layouts + +ColumnLayout { + property alias listView: listView + + ListView { + id: listView + + visible: count > 0 // actual defect. countChanged never fires so this never turns true + + Layout.fillWidth: true + Layout.preferredHeight: contentHeight // grow with content, initially 0 + + model: ListModel { + id: idModel + } + + delegate: Text { + required property string name + text: name + } + + Timer { + running: true + interval: 10 + repeat: true + onTriggered: idModel.append({name:"Hello"}) + } + } +} diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp index 96dd27e7d28..c07a30d4011 100644 --- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp +++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp @@ -18,6 +18,8 @@ Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests") using namespace QQuickViewTestUtils; using namespace QQuickVisualTestUtils; +static const int oneSecondInMs = 1000; + class tst_QQuickListView2 : public QQmlDataTest { Q_OBJECT @@ -68,6 +70,7 @@ private slots: void bindingDirectlyOnPositionInHeaderAndFooterDelegates(); void clearObjectListModel(); + void visibleBoundToCountGreaterThanZero(); private: void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to); @@ -1312,6 +1315,23 @@ void tst_QQuickListView2::clearObjectListModel() QVERIFY(!list.itemAtIndex(0)); } +void tst_QQuickListView2::visibleBoundToCountGreaterThanZero() +{ + QQuickView window; + QVERIFY(QQuickTest::showView(window, testFileUrl("visibleBoundToCountGreaterThanZero.qml"))); + + auto *listView = window.rootObject()->property("listView").value(); + QVERIFY(listView); + + QSignalSpy countChangedSpy(listView, SIGNAL(countChanged())); + QVERIFY(countChangedSpy.isValid()); + + QTRY_COMPARE_GT_WITH_TIMEOUT(listView->count(), 1, oneSecondInMs); + // Using the TRY variant here as well is necessary. + QTRY_COMPARE_GT_WITH_TIMEOUT(countChangedSpy.count(), 1, oneSecondInMs); + QVERIFY(listView->isVisible()); +} + QTEST_MAIN(tst_QQuickListView2) #include "tst_qquicklistview2.moc" -- 2.16.3