From 294e65f8092dd5b624740ee39d52979c7ca6f387 Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@digia.com>
Date: Thu, 21 Aug 2014 14:22:34 +0200
Subject: QVariant::compare shouldn't return match when QVariant::cmp does not
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If the types doesn't match in QVariant::compare we do a comparison based
on QString, this may end up indicating a full match, though the we don't
match according to cmp. In this case it would be better if we preserved
the non-matching to avoid breaking ordering.

[ChangeLog][QtCore][QVariant] Fixed ordered comparison between QVariants
that do not match but produce identical toString output.

Task-number: QTBUG-40363
Change-Id: I84a8eca11e8875dba9948bde2906ae7c5aa35704
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>
---
 src/corelib/kernel/qvariant.cpp                    | 14 ++++++++--
 .../auto/corelib/kernel/qvariant/tst_qvariant.cpp  | 30 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index 5f17b234f2..4e19e5a293 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -3198,7 +3198,12 @@ int QVariant::compare(const QVariant &v) const
         }
         if (v1.d.type != v2.d.type) {
             // if conversion fails, default to toString
-            return v1.toString().compare(v2.toString(), Qt::CaseInsensitive);
+            int r = v1.toString().compare(v2.toString(), Qt::CaseInsensitive);
+            if (r == 0) {
+                // cmp(v) returned false, so we should try to agree with it.
+                return (v1.d.type < v2.d.type) ? -1 : 1;
+            }
+            return r;
         }
     }
     if (v1.d.type >= QMetaType::User) {
@@ -3220,7 +3225,12 @@ int QVariant::compare(const QVariant &v) const
     case QVariant::DateTime:
         return v1.toDateTime() < v2.toDateTime() ? -1 : 1;
     }
-    return v1.toString().compare(v2.toString(), Qt::CaseInsensitive);
+    int r = v1.toString().compare(v2.toString(), Qt::CaseInsensitive);
+    if (r == 0) {
+        // cmp(v) returned false, so we should try to agree with it.
+        return (d.type < v.d.type) ? -1 : 1;
+    }
+    return r;
 }
 
 /*!
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index c7d862c20e..7bdc91cfbb 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -250,6 +250,10 @@ private slots:
     void pairElements();
 
     void enums();
+
+    void compareSanity_data();
+    void compareSanity();
+
 private:
     void dataStream_data(QDataStream::Version version);
     void loadQVariantFromDataStream(QDataStream::Version version);
@@ -4153,5 +4157,31 @@ void tst_QVariant::enums()
 #endif
 }
 
+void tst_QVariant::compareSanity_data()
+{
+    QTest::addColumn<QVariant>("value1");
+    QTest::addColumn<QVariant>("value2");
+
+    QTest::newRow( "int <>/== QUrl" ) << QVariant( 97 ) << QVariant(QUrl("a"));
+    QTest::newRow( "int <>/== QChar" ) << QVariant( 97 ) << QVariant(QChar('a'));
+    QTest::newRow( "int <>/== QString" ) << QVariant( 97 ) << QVariant(QString("a"));
+    QTest::newRow( "QUrl <>/== QChar" ) << QVariant(QUrl("a")) << QVariant(QChar('a'));
+    QTest::newRow( "QUrl <>/== QString" ) << QVariant(QUrl("a")) << QVariant(QString("a"));
+    QTest::newRow( "QChar <>/== QString" ) << QVariant(QChar('a')) << QVariant(QString("a"));
+}
+
+void tst_QVariant::compareSanity()
+{
+    QFETCH(QVariant, value1);
+    QFETCH(QVariant, value2);
+
+    if (value1 == value2) {
+        QVERIFY(!(value1 < value2) && !(value1 > value2));
+    } else {
+        QVERIFY(value1 != value2);
+        QVERIFY((value1 < value2) || (value1 > value2));
+    }
+}
+
 QTEST_MAIN(tst_QVariant)
 #include "tst_qvariant.moc"
-- 
cgit v1.2.3