From e9ad46ed412987e3e46c5a641e5f30408b97ac90 Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Date: Wed, 14 Jan 2015 00:25:26 +0100
Subject: [PATCH] QSortFilterProxyModel: fix a regression

bec1854cc023fb705319c582a636d5f484adafcc introduced a regression:
when sorting a tree model, children items would not follow the sorted
parents any more (they wouldn't be remapped correctly), resulting
in crashes.

So, the fix needs more reasoning; let's revert the fix,
but leave  the original test around for any subsequent attempt, and
introduce a new test which looks for the right behavior when
sorting trees.

This commit partially reverts bec1854cc023fb705319c582a636d5f484adafcc.

Task-number: QTBUG-43827
Change-Id: Ic83ac53aef639f870f6c36a8b4b2992f5b485b13
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Volker Krause <volker.krause@kdab.com>
---
 .../itemmodels/qsortfilterproxymodel.cpp      |  6 ++-
 .../tst_qsortfilterproxymodel.cpp             | 37 +++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
index e64d588432..7af95fe625 100644
--- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp
+++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
@@ -1198,7 +1198,11 @@ void QSortFilterProxyModelPrivate::_q_sourceDataChanged(const QModelIndex &sourc
         parents << q->mapFromSource(source_parent);
         emit q->layoutAboutToBeChanged(parents, QAbstractItemModel::VerticalSortHint);
         QModelIndexPairList source_indexes = store_persistent_indexes();
-        sort_source_rows(m->source_rows, source_parent);
+        remove_source_items(m->proxy_rows, m->source_rows, source_rows_resort,
+                            source_parent, Qt::Vertical, false);
+        sort_source_rows(source_rows_resort, source_parent);
+        insert_source_items(m->proxy_rows, m->source_rows, source_rows_resort,
+                            source_parent, Qt::Vertical, false);
         update_persistent_indexes(source_indexes);
         emit q->layoutChanged(parents, QAbstractItemModel::VerticalSortHint);
         // Make sure we also emit dataChanged for the rows
diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index 122427a021..645a0a724c 100644
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -97,6 +97,7 @@ private slots:
     void changeSourceData();
     void changeSourceDataKeepsStableSorting_qtbug1548();
     void changeSourceDataForwardsRoles_qtbug35440();
+    void resortingDoesNotBreakTreeModels();
     void sortFilterRole();
     void selectionFilteredOut();
     void match_data();
@@ -2030,6 +2031,8 @@ static void checkSortedTableModel(const QAbstractItemModel *model, const QString
 
 void tst_QSortFilterProxyModel::changeSourceDataKeepsStableSorting_qtbug1548()
 {
+    QSKIP("This test will fail, see QTBUG-1548");
+
     // Check that emitting dataChanged from the source model
     // for a change of a role which is not the sorting role
     // doesn't alter the sorting. In this case, we sort on the DisplayRole,
@@ -4060,5 +4063,39 @@ void tst_QSortFilterProxyModel::canDropMimeData()
         QCOMPARE(proxy.canDropMimeData(0, Qt::CopyAction, -1, -1, proxy.index(row, 0)), row < 5);
 }
 
+void tst_QSortFilterProxyModel::resortingDoesNotBreakTreeModels()
+{
+    QStandardItemModel *treeModel = new QStandardItemModel(this);
+    QStandardItem *e1 = new QStandardItem("Loading...");
+    e1->appendRow(new QStandardItem("entry10"));
+    treeModel->appendRow(e1);
+    QStandardItem *e0 = new QStandardItem("Loading...");
+    e0->appendRow(new QStandardItem("entry00"));
+    e0->appendRow(new QStandardItem("entry01"));
+    treeModel->appendRow(e0);
+
+    QSortFilterProxyModel proxy;
+    proxy.setDynamicSortFilter(true);
+    proxy.sort(0);
+    proxy.setSourceModel(treeModel);
+
+    ModelTest modelTest(&proxy);
+
+    QCOMPARE(proxy.rowCount(), 2);
+    e1->setText("entry1");
+    e0->setText("entry0");
+
+    QModelIndex pi0 = proxy.index(0, 0);
+    QCOMPARE(pi0.data().toString(), QString("entry0"));
+    QCOMPARE(proxy.rowCount(pi0), 2);
+
+    QModelIndex pi01 = proxy.index(1, 0, pi0);
+    QCOMPARE(pi01.data().toString(), QString("entry01"));
+
+    QModelIndex pi1 = proxy.index(1, 0);
+    QCOMPARE(pi1.data().toString(), QString("entry1"));
+    QCOMPARE(proxy.rowCount(pi1), 1);
+}
+
 QTEST_MAIN(tst_QSortFilterProxyModel)
 #include "tst_qsortfilterproxymodel.moc"