diff options
author | Robert Griebl <robert.griebl@qt.io> | 2023-01-23 23:51:55 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-01-26 03:21:57 +0000 |
commit | 3518b86277bc51fe8325b803c3769c6988dc05c4 (patch) | |
tree | 890143a79deb7c14b068bb4adb265b499c130c12 | |
parent | 049022370c4aa268d597993dcfb37624b732b602 (diff) |
Fix crash in ListView with PullBackHeader/Footer
The header/footer positioning code would otherwise call qBound() with
max < min when a PullBackHeader/Footer is used and the delegates do not
fill the whole content area (i.e. the list does not need scrolling).
After qtbase ad5c5bb541ae20a205ac07122153b302dee1d3e1 that was causing
an assertion failure.
Even though relying on the old qBound() behavior seems strange, it
does produce the correct header/footer positioning, while swapping
the min/max values (in case max < min) does not. So we need to call
qMin and qMax explicitly rather than using qBound, to avoid the assert.
Task-number: QTBUG-104679
Change-Id: Iefc50e347e77ed51c6d90ddbc6e1cf21d76fc62c
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit b59bc4fe935245029a96d09a7dd55b53968fcc32)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
4 files changed, 71 insertions, 2 deletions
diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index 293d8acc7c..caf1591631 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -1443,7 +1443,9 @@ void QQuickListViewPrivate::updateFooter() } else if (visibleItems.size()) { if (footerPositioning == QQuickListView::PullBackFooter) { qreal viewPos = isContentFlowReversed() ? -position() : position() + size(); - qreal clampedPos = qBound(originPosition() - footerSize() + size(), listItem->position(), lastPosition()); + // using qBound() would throw an assert here, because max < min is a valid case + // here, if the list's delegates do not fill the whole view + qreal clampedPos = qMax(originPosition() - footerSize() + size(), qMin(listItem->position(), lastPosition())); listItem->setPosition(qBound(viewPos - footerSize(), clampedPos, viewPos)); } else { qreal endPos = lastPosition(); @@ -1512,7 +1514,9 @@ void QQuickListViewPrivate::updateHeader() // Make sure the header is not shown if we absolutely do not have any plans to show it if (fixingUp && !headerNeedsSeparateFixup) headerPosition = viewPos - headerSize(); - qreal clampedPos = qBound(originPosition() - headerSize(), headerPosition, lastPosition() - size()); + // using qBound() would throw an assert here, because max < min is a valid case + // here, if the list's delegates do not fill the whole view + qreal clampedPos = qMax(originPosition() - headerSize(), qMin(headerPosition, lastPosition() - size())); listItem->setPosition(qBound(viewPos - headerSize(), clampedPos, viewPos)); } else { qreal startPos = originPosition(); diff --git a/tests/auto/quick/qquicklistview2/data/qtbug104679_footer.qml b/tests/auto/quick/qquicklistview2/data/qtbug104679_footer.qml new file mode 100644 index 0000000000..919cf4d2ec --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/qtbug104679_footer.qml @@ -0,0 +1,21 @@ +import QtQuick + +Rectangle { + width: 640 + height: 480 + + ListView { + anchors.fill: parent + spacing: 5 + + footerPositioning: ListView.PullBackFooter + footer: Rectangle { width: ListView.view.width; color: "blue"; implicitHeight: 46 } + + model: 3 // crashed if less items than a full list page + delegate: Rectangle { + width: ListView.view.width + height: 50 + color: index % 2 ? "black" : "gray" + } + } +} diff --git a/tests/auto/quick/qquicklistview2/data/qtbug104679_header.qml b/tests/auto/quick/qquicklistview2/data/qtbug104679_header.qml new file mode 100644 index 0000000000..40ddf27988 --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/qtbug104679_header.qml @@ -0,0 +1,21 @@ +import QtQuick + +Rectangle { + width: 640 + height: 480 + + ListView { + anchors.fill: parent + spacing: 5 + + headerPositioning: ListView.PullBackHeader + header: Rectangle { width: ListView.view.width; color: "red"; implicitHeight: 46 } + + model: 3 // crashed if less items than a full list page + delegate: Rectangle { + width: ListView.view.width + height: 50 + color: index % 2 ? "black" : "gray" + } + } +} diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp index e4b82d628e..98cb3cc20c 100644 --- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp +++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp @@ -50,6 +50,8 @@ private slots: void isCurrentItem_DelegateModel(); void isCurrentItem_NoRegressionWithDelegateModelGroups(); + void pullbackSparseList(); + private: void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to); QScopedPointer<QPointingDevice> touchDevice = QScopedPointer<QPointingDevice>(QTest::createTouchDevice()); @@ -909,6 +911,27 @@ void tst_QQuickListView2::isCurrentItem_NoRegressionWithDelegateModelGroups() QCOMPARE(item3->property("isCurrent").toBool(), false); } +void tst_QQuickListView2::pullbackSparseList() // QTBUG_104679 +{ + // check if PullbackHeader crashes + QScopedPointer<QQuickView> window(createView()); + QVERIFY(window); + window->setSource(testFileUrl("qtbug104679_header.qml")); + QVERIFY2(window->status() == QQuickView::Ready, qPrintable(QDebug::toString(window->errors()))); + window->resize(640, 480); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window.data())); + + // check if PullbackFooter crashes + window.reset(createView()); + QVERIFY(window); + window->setSource(testFileUrl("qtbug104679_footer.qml")); + QVERIFY2(window->status() == QQuickView::Ready, qPrintable(QDebug::toString(window->errors()))); + window->resize(640, 480); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window.data())); +} + QTEST_MAIN(tst_QQuickListView2) #include "tst_qquicklistview2.moc" |