When a move is completed, report the source and destination correctly.

In rare cases source or destination is changed during the move, so
it must be corrected afterward before being reported in the signal.

Merge-request: 800
Reviewed-by: Olivier Goffart <olivier.goffart@nokia.com>

3 files changed, 55 insertions, 10 deletions

diff --git a/src/corelib/kernel/qabstractitemmodel.cpp b/src/corelib/kernel/qabstractitemmodel.cpp
index e3fce18..9a99ea1 100644
--- a/src/corelib/kernel/qabstractitemmodel.cpp
+++ b/src/corelib/kernel/qabstractitemmodel.cpp
@@ -2603,9 +2603,13 @@ bool QAbstractItemModel::beginMoveRows(const QModelIndex &sourceParent, int sour
         return false;
     }
 
-    d->changes.push(QAbstractItemModelPrivate::Change(sourceParent, sourceFirst, sourceLast));
+    QAbstractItemModelPrivate::Change sourceChange(sourceParent, sourceFirst, sourceLast);
+    sourceChange.needsAdjust = sourceParent.isValid() && sourceParent.row() >= destinationChild && sourceParent.parent() == destinationParent;
+    d->changes.push(sourceChange);
     int destinationLast = destinationChild + (sourceLast - sourceFirst);
-    d->changes.push(QAbstractItemModelPrivate::Change(destinationParent, destinationChild, destinationLast));
+    QAbstractItemModelPrivate::Change destinationChange(destinationParent, destinationChild, destinationLast);
+    destinationChange.needsAdjust = destinationParent.isValid() && destinationParent.row() >= sourceLast && destinationParent.parent() == sourceParent;
+    d->changes.push(destinationChange);
 
     emit rowsAboutToBeMoved(sourceParent, sourceFirst, sourceLast, destinationParent, destinationChild);
     emit layoutAboutToBeChanged();
@@ -2635,7 +2639,17 @@ void QAbstractItemModel::endMoveRows()
 
     d->itemsMoved(removeChange.parent, removeChange.first, removeChange.last, insertChange.parent, insertChange.first, Qt::Vertical);
 
-    emit rowsMoved(removeChange.parent, removeChange.first, removeChange.last, insertChange.parent, insertChange.first);
+    QModelIndex adjustedSource = removeChange.parent;
+    QModelIndex adjustedDestination = insertChange.parent;
+
+    const int numMoved = removeChange.last - removeChange.first + 1;
+    if (insertChange.needsAdjust)
+      adjustedDestination = createIndex(adjustedDestination.row() - numMoved, adjustedDestination.column(), adjustedDestination.internalPointer());
+
+    if (removeChange.needsAdjust)
+      adjustedSource = createIndex(adjustedSource.row() + numMoved, adjustedSource.column(), adjustedSource.internalPointer());
+
+    emit rowsMoved(adjustedSource, removeChange.first, removeChange.last, adjustedDestination, insertChange.first);
     emit layoutChanged();
 }
 
@@ -2812,9 +2826,13 @@ bool QAbstractItemModel::beginMoveColumns(const QModelIndex &sourceParent, int s
         return false;
     }
 
-    d->changes.push(QAbstractItemModelPrivate::Change(sourceParent, sourceFirst, sourceLast));
+    QAbstractItemModelPrivate::Change sourceChange(sourceParent, sourceFirst, sourceLast);
+    sourceChange.needsAdjust = sourceParent.isValid() && sourceParent.row() >= destinationChild && sourceParent.parent() == destinationParent;
+    d->changes.push(sourceChange);
     int destinationLast = destinationChild + (sourceLast - sourceFirst);
-    d->changes.push(QAbstractItemModelPrivate::Change(destinationParent, destinationChild, destinationLast));
+    QAbstractItemModelPrivate::Change destinationChange(destinationParent, destinationChild, destinationLast);
+    destinationChange.needsAdjust = destinationParent.isValid() && destinationParent.row() >= sourceLast && destinationParent.parent() == sourceParent;
+    d->changes.push(destinationChange);
 
     d->itemsAboutToBeMoved(sourceParent, sourceFirst, sourceLast, destinationParent, destinationChild, Qt::Horizontal);
 
@@ -2845,7 +2863,17 @@ void QAbstractItemModel::endMoveColumns()
 
     d->itemsMoved(removeChange.parent, removeChange.first, removeChange.last, insertChange.parent, insertChange.first, Qt::Horizontal);
 
-    emit columnsMoved(removeChange.parent, removeChange.first, removeChange.last, insertChange.parent, insertChange.first);
+    QModelIndex adjustedSource = removeChange.parent;
+    QModelIndex adjustedDestination = insertChange.parent;
+
+    const int numMoved = removeChange.last - removeChange.first + 1;
+    if (insertChange.needsAdjust)
+      adjustedDestination = createIndex(adjustedDestination.row(), adjustedDestination.column() - numMoved, adjustedDestination.internalPointer());
+
+    if (removeChange.needsAdjust)
+      adjustedSource = createIndex(adjustedSource.row(), adjustedSource.column() + numMoved, adjustedSource.internalPointer());
+
+    emit columnsMoved(adjustedSource, removeChange.first, removeChange.last, adjustedDestination, insertChange.first);
     emit layoutChanged();
 }
 
diff --git a/src/corelib/kernel/qabstractitemmodel_p.h b/src/corelib/kernel/qabstractitemmodel_p.h
index 7844a10..3113dff 100644
--- a/src/corelib/kernel/qabstractitemmodel_p.h
+++ b/src/corelib/kernel/qabstractitemmodel_p.h
@@ -133,11 +133,26 @@ public:
 
     struct Change {
         Change() : first(-1), last(-1) {}
-        Change(const Change &c) : parent(c.parent), first(c.first), last(c.last) {}
-        Change(const QModelIndex &p, int f, int l) : parent(p), first(f), last(l) {}
+        Change(const Change &c) : parent(c.parent), first(c.first), last(c.last), needsAdjust(c.needsAdjust) {}
+        Change(const QModelIndex &p, int f, int l) : parent(p), first(f), last(l), needsAdjust(false) {}
         QModelIndex parent;
         int first, last;
 
+
+        // In cases such as this:
+        // - A
+        // - B
+        // - C
+        // - - D
+        // - - E
+        // - - F
+        //
+        // If B is moved to above E, C is the source parent in the signal and its row is 2. When the move is
+        // completed however, C is at row 1 and there is no row 2 at the same level in the model at all.
+        // The QModelIndex is adjusted to correct that in those cases before reporting it though the
+        // rowsMoved signal.
+        bool needsAdjust;
+
         bool isValid() { return first >= 0 && last >= 0; }
     };
     QStack<Change> changes;
diff --git a/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp b/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
index 6f158f0..dbcccc9 100644
--- a/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
+++ b/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
@@ -1208,6 +1208,7 @@ void tst_QAbstractItemModel::testMoveToGrandParent()
     QSignalSpy beforeSpy(m_model, SIGNAL(rowsAboutToBeMoved(const QModelIndex &, int, int, const QModelIndex &, int)));
     QSignalSpy afterSpy(m_model, SIGNAL(rowsMoved(const QModelIndex &, int, int, const QModelIndex &, int)));
 
+    QPersistentModelIndex persistentSource = sourceIndex;
 
     ModelMoveCommand *moveCommand = new ModelMoveCommand(m_model, this);
     moveCommand->setAncestorRowNumbers(QList<int>() << 5);
@@ -1228,7 +1229,7 @@ void tst_QAbstractItemModel::testMoveToGrandParent()
     QCOMPARE(beforeSignal.at(4).toInt(), destRow);
 
     QCOMPARE(afterSignal.size(), 5);
-    QCOMPARE(afterSignal.at(0).value<QModelIndex>(), sourceIndex);
+    QCOMPARE(afterSignal.at(0).value<QModelIndex>(), static_cast<QModelIndex>(persistentSource));
     QCOMPARE(afterSignal.at(1).toInt(), startRow);
     QCOMPARE(afterSignal.at(2).toInt(), endRow);
     QCOMPARE(afterSignal.at(3).value<QModelIndex>(), QModelIndex());
@@ -1351,6 +1352,7 @@ void tst_QAbstractItemModel::testMoveToSibling()
     QSignalSpy beforeSpy(m_model, SIGNAL(rowsAboutToBeMoved(const QModelIndex &, int, int, const QModelIndex &, int)));
     QSignalSpy afterSpy(m_model, SIGNAL(rowsMoved(const QModelIndex &, int, int, const QModelIndex &, int)));
 
+    QPersistentModelIndex persistentDest = destIndex;
 
     ModelMoveCommand *moveCommand = new ModelMoveCommand(m_model, this);
     moveCommand->setNumCols(4);
@@ -1374,7 +1376,7 @@ void tst_QAbstractItemModel::testMoveToSibling()
     QCOMPARE(afterSignal.at(0).value<QModelIndex>(), sourceIndex);
     QCOMPARE(afterSignal.at(1).toInt(), startRow);
     QCOMPARE(afterSignal.at(2).toInt(), endRow);
-    QCOMPARE(afterSignal.at(3).value<QModelIndex>(), destIndex);
+    QCOMPARE(afterSignal.at(3).value<QModelIndex>(), static_cast<QModelIndex>(persistentDest));
     QCOMPARE(afterSignal.at(4).toInt(), destRow);
 
     for (int i = 0; i < indexList.size(); i++)