QFileSystemModel: fix sorting

When sorting a model recursively, the children of a QFileSystemNode
are extracted from their parent in a QHash order; then filtered,
then sorted (using a stable sort) depending on the sorting column.

This means that the order of the children comparing to equal for
the chosen sort are shown in the order they were picked from the
iteration on the QHash, which isn't reliable at all.

Moreover, the criteria used in QFileSystemModelSorter for sorting
are too loose: when sorting by any column but the name, if the result
is "equality", then the file names should be used to determine
the sort order.

This patch removes the stable sort in favour of a full sort,
and fixes the criteria of soring inside QFileSystemModelSorter.

Change-Id: Idd9aece22f2ebbe77ec40d372b43cde4c200ff38
Reviewed-by: Stephen Kelly <stephen.kelly@kdab.com>

diff --git a/src/widgets/dialogs/qfilesystemmodel.cpp b/src/widgets/dialogs/qfilesystemmodel.cpp
index 5446eca..809024a 100644 (file)
--- a/src/widgets/dialogs/qfilesystemmodel.cpp
+++ b/src/widgets/dialogs/qfilesystemmodel.cpp
@@ -48,6 +48,8 @@
 #include <qmessagebox.h>
 #include <qapplication.h>
 
+#include <algorithm>
+
 #ifdef Q_OS_WIN
 #  include <QtCore/QVarLengthArray>
 #  include <qt_windows.h>
@@ -1081,15 +1083,35 @@ public:
                                                 r->fileName, Qt::CaseInsensitive) < 0;
                 }
         case 1:
+        {
             // Directories go first
-            if (l->isDir() && !r->isDir())
-                return true;
-            return l->size() < r->size();
+            bool left = l->isDir();
+            bool right = r->isDir();
+            if (left ^ right)
+                return left;
+
+            qint64 sizeDifference = l->size() - r->size();
+            if (sizeDifference == 0)
+                return QFileSystemModelPrivate::naturalCompare(l->fileName, r->fileName, Qt::CaseInsensitive) < 0;
+
+            return sizeDifference < 0;
+        }
         case 2:
-            return l->type() < r->type();
+        {
+            int compare = QString::localeAwareCompare(l->type(), r->type());
+            if (compare == 0)
+                return QFileSystemModelPrivate::naturalCompare(l->fileName, r->fileName, Qt::CaseInsensitive) < 0;
+
+            return compare < 0;
+        }
         case 3:
+        {
+            if (l->lastModified() == r->lastModified())
+                return QFileSystemModelPrivate::naturalCompare(l->fileName, r->fileName, Qt::CaseInsensitive) < 0;
+
             return l->lastModified() < r->lastModified();
         }
+        }
         Q_ASSERT(false);
         return false;
     }
@@ -1129,7 +1151,7 @@ void QFileSystemModelPrivate::sortChildren(int column, const QModelIndex &parent
         i++;
     }
     QFileSystemModelSorter ms(column);
-    qStableSort(values.begin(), values.end(), ms);
+    std::sort(values.begin(), values.end(), ms);
     // First update the new visible list
     indexNode->visibleChildren.clear();
     //No more dirty item we reset our internal dirty index

diff --git a/tests/auto/widgets/dialogs/qfilesystemmodel/tst_qfilesystemmodel.cpp b/tests/auto/widgets/dialogs/qfilesystemmodel/tst_qfilesystemmodel.cpp
index 1a3d083..5eaf8b1 100644 (file)
--- a/tests/auto/widgets/dialogs/qfilesystemmodel/tst_qfilesystemmodel.cpp
+++ b/tests/auto/widgets/dialogs/qfilesystemmodel/tst_qfilesystemmodel.cpp
@@ -863,7 +863,7 @@ void tst_QFileSystemModel::sort()
     QTest::qWait(500);
     QModelIndex parent = myModel->index(dirPath, 0);
     QList<QString> expectedOrder;
-    expectedOrder << tempFile2.fileName() << tempFile.fileName() << dirPath + QChar('/') + "." << dirPath + QChar('/') + "..";
+    expectedOrder << tempFile2.fileName() << tempFile.fileName() << dirPath + QChar('/') + ".." << dirPath + QChar('/') + ".";
     //File dialog Mode means sub trees are not sorted, only the current root
     if (fileDialogMode) {
        // FIXME: we were only able to disableRecursiveSort in developer builds, so we can only