Fix less-than comparison for QStandardItem and QSortFilterProxyModel with invalid data.

Previously, QStandardItem::operator<() returned true when both
items had invalid data. With MSVC in debug mode (checked
iterators/STL), this triggered an assert in
tst_QStandardItem::sortChildren() since that verifies
that !(b < a) when a < b:

Debug Assertion Failed!
Line: 3006
Expression: invalid operator<

Introduce a stable sort order for invalid items such that
other items are always less than invalid items and comparing
invalid items returns false (indicating equivalence).

Change-Id: Ica0f0d9f001c86973b1941dbcc1faf282e4c47df
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>

4 files changed, 30 insertions, 10 deletions

diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
index 922d0f1622..38f8dfa788 100644
--- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp
+++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
@@ -2617,9 +2617,12 @@ bool QSortFilterProxyModel::lessThan(const QModelIndex &source_left, const QMode
     Q_D(const QSortFilterProxyModel);
     QVariant l = (source_left.model() ? source_left.model()->data(source_left, d->sort_role) : QVariant());
     QVariant r = (source_right.model() ? source_right.model()->data(source_right, d->sort_role) : QVariant());
+    // Duplicated in QStandardItem::operator<()
+    if (l.userType() == QVariant::Invalid)
+        return false;
+    if (r.userType() == QVariant::Invalid)
+        return true;
     switch (l.userType()) {
-    case QVariant::Invalid:
-        return (r.type() != QVariant::Invalid);
     case QVariant::Int:
         return l.toInt() < r.toInt();
     case QVariant::UInt:
diff --git a/src/gui/itemmodels/qstandarditemmodel.cpp b/src/gui/itemmodels/qstandarditemmodel.cpp
index ed2479302e..8d848b662f 100644
--- a/src/gui/itemmodels/qstandarditemmodel.cpp
+++ b/src/gui/itemmodels/qstandarditemmodel.cpp
@@ -1811,9 +1811,11 @@ bool QStandardItem::operator<(const QStandardItem &other) const
     const int role = model() ? model()->sortRole() : Qt::DisplayRole;
     const QVariant l = data(role), r = other.data(role);
     // this code is copied from QSortFilterProxyModel::lessThan()
+    if (l.userType() == QVariant::Invalid)
+        return false;
+    if (r.userType() == QVariant::Invalid)
+        return true;
     switch (l.userType()) {
-    case QVariant::Invalid:
-        return (r.type() == QVariant::Invalid);
     case QVariant::Int:
         return l.toInt() < r.toInt();
     case QVariant::UInt:
diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index 01c10886c5..1c30adc8de 100644
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -2502,16 +2502,16 @@ void tst_QSortFilterProxyModel::sortColumnTracking2()
     proxyModel.sort(0);
     QCOMPARE(proxyModel.sortColumn(), 0);
 
-    QList<QStandardItem *> items;
-    QStringList strings;
-    strings << "foo" << "bar" << "some" << "others" << "item" << "aa" << "zz";
-    foreach (QString s, strings)
-        items  << new QStandardItem(s);
+    QList<QStandardItem *> items; // Stable sorting: Items with invalid data should move to the end
+    items << new QStandardItem << new QStandardItem("foo") << new QStandardItem("bar")
+        << new QStandardItem("some") << new QStandardItem("others") << new QStandardItem("item")
+        << new QStandardItem("aa") << new QStandardItem("zz") << new QStandardItem;
 
     model.insertColumn(0,items);
     QCOMPARE(proxyModel.sortColumn(), 0);
     QCOMPARE(proxyModel.data(proxyModel.index(0,0)).toString(),QString::fromLatin1("aa"));
-    QCOMPARE(proxyModel.data(proxyModel.index(strings.count()-1,0)).toString(),QString::fromLatin1("zz"));
+    const int zzIndex = items.count() - 3; // 2 invalid at end.
+    QCOMPARE(proxyModel.data(proxyModel.index(zzIndex,0)).toString(),QString::fromLatin1("zz"));
 }
 
 void tst_QSortFilterProxyModel::sortStable()
diff --git a/tests/auto/gui/itemmodels/qstandarditem/tst_qstandarditem.cpp b/tests/auto/gui/itemmodels/qstandarditem/tst_qstandarditem.cpp
index 5dce228ca0..2448b89296 100644
--- a/tests/auto/gui/itemmodels/qstandarditem/tst_qstandarditem.cpp
+++ b/tests/auto/gui/itemmodels/qstandarditem/tst_qstandarditem.cpp
@@ -81,6 +81,7 @@ private slots:
     void clone();
     void sortChildren();
     void subclassing();
+    void lessThan();
 };
 
 tst_QStandardItem::tst_QStandardItem()
@@ -1095,5 +1096,19 @@ void tst_QStandardItem::subclassing()
     QCOMPARE(item->child(2), (QStandardItem*)child1);
 }
 
+void tst_QStandardItem::lessThan()
+{
+    QStandardItem stringA("A");
+    QStandardItem stringB("B");
+    QStandardItem invalid1;
+    QStandardItem invalid2;
+    QVERIFY(stringA < stringB);
+    QVERIFY(!(stringB < stringA));
+    // Items with invalid data go to the end.
+    QVERIFY(stringA < invalid1);
+    QVERIFY(!(invalid1 < stringA));
+    QVERIFY(!(invalid1 < invalid2));
+}
+
 QTEST_MAIN(tst_QStandardItem)
 #include "tst_qstandarditem.moc"