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>
(cherry-picked from qtbase/e9ad46ed412987e3e46c5a641e5f30408b97ac90)

diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
index 0b2b0e41880e1fd3266dec81a73934a5cb8fd6a7..b01c9db4186bae982545a98c50161217b4c6f4e8 100644 (file)
--- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp
+++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
@@ -1196,7 +1196,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 53ed1bc9a068e344b38e4ebb7b0f6e3c23f01340..5bb7ffc401779fc1e21260b43f079d2f9638f760 100644 (file)
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -96,6 +96,7 @@ private slots:
     void changeSourceData_data();
     void changeSourceData();
     void changeSourceDataKeepsStableSorting_qtbug1548();
+    void resortingDoesNotBreakTreeModels();
     void sortFilterRole();
     void selectionFilteredOut();
     void match_data();
@@ -2029,6 +2030,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,
@@ -4028,5 +4031,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"