diff options
author | Santhosh Kumar <santhosh.kumar.selvaraj@qt.io> | 2023-11-02 22:14:31 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-11-13 22:22:44 +0000 |
commit | d02351d8debe3c089434ff6baf2caff6be52602a (patch) | |
tree | 3a9588d1ffa1a7b040837b3a129704a0f8c5ef22 | |
parent | 642b08d15726b461864d6de7246e085f5d046b68 (diff) |
Fix crash issue in quick table view control
The quick table view can have both QAbstractItemModel and JS value
as its model. The table view release and load the items (or rebuild)
when its assigned with new model. The newmodel always be compared
with existing model before rebuild via
QQuickTableViewPrivate::syncModel. This comparison works with
QAbstractItemModel but the same fails with JS model.
This patch adds additional validation in
QQuickTableViewPrivate::syncModel and QQuickTableViewPrivate::setModel
for comparing JS model to avoid rebuild when existing model
overwritten with same model again.
Fixes: QTBUG-117917
Pick-to: 6.5 6.2
Change-Id: Ic15145c4b8998c68ae6471a2abf4aef727041eea
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 79b34126850c4235a1914e95f8e4235cfddba007)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/quick/items/qquicktableview.cpp | 14 | ||||
-rw-r--r-- | src/quick/items/qquicktableview_p_p.h | 1 | ||||
-rw-r--r-- | src/quick/items/qquicktreeview.cpp | 3 | ||||
-rw-r--r-- | tests/auto/quick/qquicktableview/data/resetJsModelData.qml | 19 | ||||
-rw-r--r-- | tests/auto/quick/qquicktableview/tst_qquicktableview.cpp | 27 |
5 files changed, 57 insertions, 7 deletions
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index d986f59d3a..2e2be92006 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -4288,9 +4288,6 @@ QVariant QQuickTableViewPrivate::modelImpl() const void QQuickTableViewPrivate::setModelImpl(const QVariant &newModel) { - if (newModel == assignedModel) - return; - assignedModel = newModel; scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::All); emit q_func()->modelChanged(); @@ -4298,7 +4295,7 @@ void QQuickTableViewPrivate::setModelImpl(const QVariant &newModel) void QQuickTableViewPrivate::syncModel() { - if (modelVariant == assignedModel) + if (compareModel(modelVariant, assignedModel)) return; if (model) { @@ -4574,6 +4571,13 @@ void QQuickTableViewPrivate::modelResetCallback() scheduleRebuildTable(RebuildOption::All); } +bool QQuickTableViewPrivate::compareModel(const QVariant& model1, const QVariant& model2) const +{ + return (model1 == model2 || + (model1.userType() == qMetaTypeId<QJSValue>() && model2.userType() == qMetaTypeId<QJSValue>() && + model1.value<QJSValue>().strictlyEquals(model2.value<QJSValue>()))); +} + void QQuickTableViewPrivate::positionViewAtRow(int row, Qt::Alignment alignment, qreal offset, const QRectF subRect) { Qt::Alignment verticalAlignment = alignment & (Qt::AlignTop | Qt::AlignVCenter | Qt::AlignBottom); @@ -5437,6 +5441,8 @@ QVariant QQuickTableView::model() const void QQuickTableView::setModel(const QVariant &newModel) { Q_D(QQuickTableView); + if (d->compareModel(newModel, d->assignedModel)) + return; closeEditor(); d->setModelImpl(newModel); diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h index b1431af009..60ccb26c9e 100644 --- a/src/quick/items/qquicktableview_p_p.h +++ b/src/quick/items/qquicktableview_p_p.h @@ -534,6 +534,7 @@ public: void columnsRemovedCallback(const QModelIndex &parent, int begin, int end); void layoutChangedCallback(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint); void modelResetCallback(); + bool compareModel(const QVariant& model1, const QVariant& model2) const; void positionViewAtRow(int row, Qt::Alignment alignment, qreal offset, const QRectF subRect = QRectF()); void positionViewAtColumn(int column, Qt::Alignment alignment, qreal offset, const QRectF subRect = QRectF()); diff --git a/src/quick/items/qquicktreeview.cpp b/src/quick/items/qquicktreeview.cpp index 521adcadce..488e9eaa44 100644 --- a/src/quick/items/qquicktreeview.cpp +++ b/src/quick/items/qquicktreeview.cpp @@ -278,9 +278,6 @@ void QQuickTreeViewPrivate::setModelImpl(const QVariant &newModel) { Q_Q(QQuickTreeView); - if (newModel == m_assignedModel) - return; - m_assignedModel = newModel; QVariant effectiveModel = m_assignedModel; if (effectiveModel.userType() == qMetaTypeId<QJSValue>()) diff --git a/tests/auto/quick/qquicktableview/data/resetJsModelData.qml b/tests/auto/quick/qquicktableview/data/resetJsModelData.qml new file mode 100644 index 0000000000..def5346147 --- /dev/null +++ b/tests/auto/quick/qquicktableview/data/resetJsModelData.qml @@ -0,0 +1,19 @@ +import QtQuick + +Item { + width: 100 + height: 300 + + property alias tableView: tableView + + TableView { + id: tableView + anchors.fill: parent + property int modelUpdated: 0 + onModelChanged: { ++modelUpdated } + delegate: Item { + implicitHeight: 10 + implicitWidth: 10 + } + } +} diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp index 215966e4f0..96137877cb 100644 --- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp +++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp @@ -277,6 +277,7 @@ private slots: void resettingRolesRespected(); void checkScroll_data(); void checkScroll(); + void checkRebuildJsModel(); }; tst_QQuickTableView::tst_QQuickTableView() @@ -7496,6 +7497,32 @@ void tst_QQuickTableView::checkScroll() // QTBUG-116566 QTRY_COMPARE_GT(tableView->contentY(), 20); } +void tst_QQuickTableView::checkRebuildJsModel() +{ + LOAD_TABLEVIEW("resetJsModelData.qml"); // gives us 'tableView' variable + + // Generate javascript model + const int size = 5; + const char* modelUpdated = "modelUpdated"; + + QJSEngine jsEngine; + QJSValue jsArray; + jsArray = jsEngine.newArray(size); + for (int i = 0; i < size; ++i) + jsArray.setProperty(i, QRandomGenerator::global()->generate()); + + QVariant jsModel = QVariant::fromValue(jsArray); + tableView->setModel(jsModel); + WAIT_UNTIL_POLISHED; + + // Model change would be triggered for the first time + QCOMPARE(tableView->property(modelUpdated).toInt(), 1); + + // Set the same model once again and check if model changes + tableView->setModel(jsModel); + QCOMPARE(tableView->property(modelUpdated).toInt(), 1); +} + QTEST_MAIN(tst_QQuickTableView) #include "tst_qquicktableview.moc" |