From 440fb56b3095b96b08b5401568eca4c3307270a0 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Mon, 29 Aug 2011 13:58:05 +1000 Subject: [PATCH] Fix conversion of QAbstractItemModel::rowsMoved() parameters VisualDataModel was emitting itemsMoved() signal with incorrect "to" value when multiple items were moved forwards. (change cherry picked from b64817858484db6e7c280d41ed81d4c87dff2275) Change-Id: I684eb1716d2ad6e0f702e95d9c0cd5c6fe2c9cbb Reviewed-on: http://codereview.qt.nokia.com/3694 Reviewed-by: Bea Lam Reviewed-by: Qt Sanity Bot --- src/declarative/items/qsgvisualitemmodel.cpp | 2 +- .../graphicsitems/qdeclarativevisualitemmodel.cpp | 2 +- .../qsgvisualdatamodel/tst_qsgvisualdatamodel.cpp | 79 +++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/declarative/items/qsgvisualitemmodel.cpp b/src/declarative/items/qsgvisualitemmodel.cpp index 5dfe01d..4dd508d 100644 --- a/src/declarative/items/qsgvisualitemmodel.cpp +++ b/src/declarative/items/qsgvisualitemmodel.cpp @@ -1511,7 +1511,7 @@ void QSGVisualDataModel::_q_rowsMoved(const QModelIndex &sourceParent, int sourc Q_D(QSGVisualDataModel); const int count = sourceEnd - sourceStart + 1; if (destinationParent == d->m_root && sourceParent == d->m_root) { - _q_itemsMoved(sourceStart, sourceStart > destinationRow ? destinationRow : destinationRow-1, count); + _q_itemsMoved(sourceStart, sourceStart > destinationRow ? destinationRow : destinationRow-count, count); } else if (sourceParent == d->m_root) { _q_itemsRemoved(sourceStart, count); } else if (destinationParent == d->m_root) { diff --git a/src/qtquick1/graphicsitems/qdeclarativevisualitemmodel.cpp b/src/qtquick1/graphicsitems/qdeclarativevisualitemmodel.cpp index 5d8a3d4..f76a4d9 100644 --- a/src/qtquick1/graphicsitems/qdeclarativevisualitemmodel.cpp +++ b/src/qtquick1/graphicsitems/qdeclarativevisualitemmodel.cpp @@ -1379,7 +1379,7 @@ void QDeclarative1VisualDataModel::_q_rowsMoved(const QModelIndex &sourceParent, Q_D(QDeclarative1VisualDataModel); const int count = sourceEnd - sourceStart + 1; if (destinationParent == d->m_root && sourceParent == d->m_root) { - _q_itemsMoved(sourceStart, sourceStart > destinationRow ? destinationRow : destinationRow-1, count); + _q_itemsMoved(sourceStart, sourceStart > destinationRow ? destinationRow : destinationRow-count, count); } else if (sourceParent == d->m_root) { _q_itemsRemoved(sourceStart, count); } else if (destinationParent == d->m_root) { diff --git a/tests/auto/declarative/qsgvisualdatamodel/tst_qsgvisualdatamodel.cpp b/tests/auto/declarative/qsgvisualdatamodel/tst_qsgvisualdatamodel.cpp index 7470153..40fddef 100644 --- a/tests/auto/declarative/qsgvisualdatamodel/tst_qsgvisualdatamodel.cpp +++ b/tests/auto/declarative/qsgvisualdatamodel/tst_qsgvisualdatamodel.cpp @@ -38,6 +38,7 @@ ** $QT_END_LICENSE$ ** ****************************************************************************/ +#include "../../../shared/util.h" #include #include #include @@ -88,6 +89,13 @@ public: list << "one" << "two" << "three" << "four"; } + void emitMove(int sourceFirst, int sourceLast, int destinationChild) { + emit beginMoveRows(QModelIndex(), sourceFirst, sourceLast, QModelIndex(), destinationChild); + emit endMoveRows(); + } + + QStringList list; + public slots: void set(int idx, QString string) { list[idx] = string; @@ -103,9 +111,6 @@ protected: return list.at(index.row()); return QVariant(); } - -private: - QStringList list; }; @@ -125,6 +130,8 @@ private slots: void singleRole(); void modelProperties(); void noDelegate(); + void qaimRowsMoved(); + void qaimRowsMoved_data(); private: QDeclarativeEngine engine; @@ -538,6 +545,72 @@ void tst_qsgvisualdatamodel::noDelegate() } +void tst_qsgvisualdatamodel::qaimRowsMoved() +{ + // Test parameters passed in QAIM::rowsMoved() signal are converted correctly + // when translated and emitted as the QListModelInterface::itemsMoved() signal + QFETCH(int, sourceFirst); + QFETCH(int, sourceLast); + QFETCH(int, destinationChild); + QFETCH(int, expectFrom); + QFETCH(int, expectTo); + QFETCH(int, expectCount); + + QDeclarativeEngine engine; + QDeclarativeComponent c(&engine, QUrl::fromLocalFile(SRCDIR "/data/visualdatamodel.qml")); + + SingleRoleModel model; + model.list.clear(); + for (int i=0; i<30; i++) + model.list << ("item " + i); + engine.rootContext()->setContextProperty("myModel", &model); + + QSGVisualDataModel *obj = qobject_cast(c.create()); + QVERIFY(obj != 0); + + QSignalSpy spy(obj, SIGNAL(itemsMoved(int,int,int))); + model.emitMove(sourceFirst, sourceLast, destinationChild); + QTRY_COMPARE(spy.count(), 1); + + QCOMPARE(spy[0].count(), 3); + QCOMPARE(spy[0][0].toInt(), expectFrom); + QCOMPARE(spy[0][1].toInt(), expectTo); + QCOMPARE(spy[0][2].toInt(), expectCount); + + delete obj; +} + +void tst_qsgvisualdatamodel::qaimRowsMoved_data() +{ + QTest::addColumn("sourceFirst"); + QTest::addColumn("sourceLast"); + QTest::addColumn("destinationChild"); + QTest::addColumn("expectFrom"); + QTest::addColumn("expectTo"); + QTest::addColumn("expectCount"); + + QTest::newRow("move 1 forward") + << 1 << 1 << 6 + << 1 << 5 << 1; + + QTest::newRow("move 1 backwards") + << 4 << 4 << 1 + << 4 << 1 << 1; + + QTest::newRow("move multiple forwards") + << 0 << 2 << 13 + << 0 << 10 << 3; + + QTest::newRow("move multiple forwards, with same to") + << 0 << 1 << 3 + << 0 << 1 << 2; + + QTest::newRow("move multiple backwards") + << 10 << 14 << 1 + << 10 << 1 << 5; +} + + template T *tst_qsgvisualdatamodel::findItem(QSGItem *parent, const QString &objectName, int index) { -- 1.7.2.5