From cf959b4b4ea3d2dfd5243022fea393fadfd95b0d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 29 Oct 2014 13:59:46 +0100 Subject: Repeater & itemviews: fix setModel() JS array handling QVariant comparison in setModel() started failing because JS arrays are now passed as a QJSValue. Re-assigning the same array content should not trigger a model change. This change restores the old behavior it had before, when JS arrays were passed as QVariantLists. Change-Id: I1882b3531f2893b116dbd817edeecab1ae812ce8 Reviewed-by: Simon Hausmann --- src/quick/items/qquickitemview.cpp | 6 ++++- src/quick/items/qquickpathview.cpp | 6 ++++- src/quick/items/qquickrepeater.cpp | 6 ++++- .../quick/qquickgridview/tst_qquickgridview.cpp | 29 ++++++++++++++++++++++ .../quick/qquicklistview/tst_qquicklistview.cpp | 29 ++++++++++++++++++++++ .../quick/qquickpathview/tst_qquickpathview.cpp | 28 +++++++++++++++++++++ .../quick/qquickrepeater/tst_qquickrepeater.cpp | 28 +++++++++++++++++++++ 7 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 2fd79715e1..93cb5e4e9d 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -273,9 +273,13 @@ QVariant QQuickItemView::model() const return d->modelVariant; } -void QQuickItemView::setModel(const QVariant &model) +void QQuickItemView::setModel(const QVariant &m) { Q_D(QQuickItemView); + QVariant model = m; + if (model.userType() == qMetaTypeId()) + model = model.value().toVariant(); + if (d->modelVariant == model) return; if (d->model) { diff --git a/src/quick/items/qquickpathview.cpp b/src/quick/items/qquickpathview.cpp index 825845eca9..6cf3e33de9 100644 --- a/src/quick/items/qquickpathview.cpp +++ b/src/quick/items/qquickpathview.cpp @@ -607,9 +607,13 @@ QVariant QQuickPathView::model() const return d->modelVariant; } -void QQuickPathView::setModel(const QVariant &model) +void QQuickPathView::setModel(const QVariant &m) { Q_D(QQuickPathView); + QVariant model = m; + if (model.userType() == qMetaTypeId()) + model = model.value().toVariant(); + if (d->modelVariant == model) return; diff --git a/src/quick/items/qquickrepeater.cpp b/src/quick/items/qquickrepeater.cpp index 8e13947d78..e2a3043857 100644 --- a/src/quick/items/qquickrepeater.cpp +++ b/src/quick/items/qquickrepeater.cpp @@ -183,9 +183,13 @@ QVariant QQuickRepeater::model() const return d->dataSource; } -void QQuickRepeater::setModel(const QVariant &model) +void QQuickRepeater::setModel(const QVariant &m) { Q_D(QQuickRepeater); + QVariant model = m; + if (model.userType() == qMetaTypeId()) + model = model.value().toVariant(); + if (d->dataSource == model) return; diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index b65aad543d..123d7f5032 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -204,6 +204,8 @@ private slots: void displayMargin(); void negativeDisplayMargin(); + void jsArrayChange(); + private: QList toIntList(const QVariantList &list); void matchIndexLists(const QVariantList &indexLists, const QList &expectedIndexes); @@ -6430,6 +6432,33 @@ void tst_QQuickGridView::negativeDisplayMargin() delete window; } +void tst_QQuickGridView::jsArrayChange() +{ + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData("import QtQuick 2.4; GridView {}", QUrl()); + + QScopedPointer view(qobject_cast(component.create())); + QVERIFY(!view.isNull()); + + QSignalSpy spy(view.data(), SIGNAL(modelChanged())); + QVERIFY(spy.isValid()); + + QJSValue array1 = engine.newArray(3); + QJSValue array2 = engine.newArray(3); + for (int i = 0; i < 3; ++i) { + array1.setProperty(i, i); + array2.setProperty(i, i); + } + + view->setModel(QVariant::fromValue(array1)); + QCOMPARE(spy.count(), 1); + + // no change + view->setModel(QVariant::fromValue(array2)); + QCOMPARE(spy.count(), 1); +} + QTEST_MAIN(tst_QQuickGridView) #include "tst_qquickgridview.moc" diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index 5122b6aa38..9e8a813d54 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -239,6 +239,8 @@ private slots: void QTBUG_39492_data(); void QTBUG_39492(); + void jsArrayChange(); + private: template void items(const QUrl &source); template void changed(const QUrl &source); @@ -7945,6 +7947,33 @@ void tst_QQuickListView::QTBUG_39492() releaseView(window); } +void tst_QQuickListView::jsArrayChange() +{ + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData("import QtQuick 2.4; ListView {}", QUrl()); + + QScopedPointer view(qobject_cast(component.create())); + QVERIFY(!view.isNull()); + + QSignalSpy spy(view.data(), SIGNAL(modelChanged())); + QVERIFY(spy.isValid()); + + QJSValue array1 = engine.newArray(3); + QJSValue array2 = engine.newArray(3); + for (int i = 0; i < 3; ++i) { + array1.setProperty(i, i); + array2.setProperty(i, i); + } + + view->setModel(QVariant::fromValue(array1)); + QCOMPARE(spy.count(), 1); + + // no change + view->setModel(QVariant::fromValue(array2)); + QCOMPARE(spy.count(), 1); +} + QTEST_MAIN(tst_QQuickListView) #include "tst_qquicklistview.moc" diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp index 0e9994899f..992b2347dd 100644 --- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp +++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp @@ -139,6 +139,7 @@ private slots: void changePathDuringRefill(); void nestedinFlickable(); void flickableDelegate(); + void jsArrayChange(); }; class TestObject : public QObject @@ -2290,6 +2291,33 @@ void tst_QQuickPathView::flickableDelegate() QCOMPARE(moveStartedSpy.count(), 0); } +void tst_QQuickPathView::jsArrayChange() +{ + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData("import QtQuick 2.4; PathView {}", QUrl()); + + QScopedPointer view(qobject_cast(component.create())); + QVERIFY(!view.isNull()); + + QSignalSpy spy(view.data(), SIGNAL(modelChanged())); + QVERIFY(spy.isValid()); + + QJSValue array1 = engine.newArray(3); + QJSValue array2 = engine.newArray(3); + for (int i = 0; i < 3; ++i) { + array1.setProperty(i, i); + array2.setProperty(i, i); + } + + view->setModel(QVariant::fromValue(array1)); + QCOMPARE(spy.count(), 1); + + // no change + view->setModel(QVariant::fromValue(array2)); + QCOMPARE(spy.count(), 1); +} + QTEST_MAIN(tst_QQuickPathView) #include "tst_qquickpathview.moc" diff --git a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp index 1396dd4783..258eaee981 100644 --- a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp +++ b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp @@ -72,6 +72,7 @@ private slots: void dynamicModelCrash(); void visualItemModelCrash(); void invalidContextCrash(); + void jsArrayChange(); }; class TestObject : public QObject @@ -779,6 +780,33 @@ void tst_QQuickRepeater::invalidContextCrash() root.reset(0); } +void tst_QQuickRepeater::jsArrayChange() +{ + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData("import QtQuick 2.4; Repeater {}", QUrl()); + + QScopedPointer repeater(qobject_cast(component.create())); + QVERIFY(!repeater.isNull()); + + QSignalSpy spy(repeater.data(), SIGNAL(modelChanged())); + QVERIFY(spy.isValid()); + + QJSValue array1 = engine.newArray(3); + QJSValue array2 = engine.newArray(3); + for (int i = 0; i < 3; ++i) { + array1.setProperty(i, i); + array2.setProperty(i, i); + } + + repeater->setModel(QVariant::fromValue(array1)); + QCOMPARE(spy.count(), 1); + + // no change + repeater->setModel(QVariant::fromValue(array2)); + QCOMPARE(spy.count(), 1); +} + QTEST_MAIN(tst_QQuickRepeater) #include "tst_qquickrepeater.moc" -- cgit v1.2.3