QML Binding: do not convert strings

The root cause for the issue is that QQmlObjectCreator::setPropertyValue
calls QQmlStringConverters::variantFromString on strings if the property
is of type QVariant. Unfortunately, this cannot be changed easily as the
current behavior is explicitly documented and tested in tst_qqmllanguage,
thus making it a breaking change.

As a workaround, QML Binding does now take a QJSValue instead of a
QVariant (making value a var property), which does not trigger the
conversion path.

Fixes: QTBUG-78943
Change-Id: I0b64dffdb6b84b2bab2bb85a8cb263e530c18570
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 60 insertions, 34 deletions

diff --git a/tests/auto/qml/qqmlbinding/data/noUnexpectedStringConversion.qml b/tests/auto/qml/qqmlbinding/data/noUnexpectedStringConversion.qml
new file mode 100644
index 0000000000..d0f30c5da5
--- /dev/null
+++ b/tests/auto/qml/qqmlbinding/data/noUnexpectedStringConversion.qml
@@ -0,0 +1,30 @@
+import QtQuick 2.12
+import QtQuick.Window 2.12
+
+Window {
+visible: true
+width: 640
+height: 480
+title: qsTr("Hello World")
+
+    Rectangle {
+        id: colorRect
+        objectName: "colorRect"
+        anchors.fill: parent
+        Text {
+            objectName: "colorLabel"
+            id: colorLabel
+        }
+    }
+
+    Binding {
+        target: colorLabel
+        property: "text"
+        value: "red"
+    }
+    Binding {
+        target: colorRect
+        property: "color"
+        value: "red"
+    }
+}
diff --git a/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp b/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
index 9b66cd828a..2c2d311ff7 100644
--- a/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
+++ b/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
@@ -55,6 +55,7 @@ private slots:
     void delayed();
     void bindingOverwriting();
     void bindToQmlComponent();
+    void bindingDoesNoWeirdConversion();
 
 private:
     QQmlEngine engine;
@@ -68,7 +69,7 @@ void tst_qqmlbinding::binding()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("test-binding.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect { qobject_cast<QQuickRectangle*>(c.create()) };
     QVERIFY(rect != nullptr);
 
     QQmlBind *binding3 = qobject_cast<QQmlBind*>(rect->findChild<QQmlBind*>("binding3"));
@@ -85,18 +86,16 @@ void tst_qqmlbinding::binding()
 
     QQmlBind *binding = qobject_cast<QQmlBind*>(rect->findChild<QQmlBind*>("binding1"));
     QVERIFY(binding != nullptr);
-    QCOMPARE(binding->object(), qobject_cast<QObject*>(rect));
+    QCOMPARE(binding->object(), qobject_cast<QObject*>(rect.get()));
     QCOMPARE(binding->property(), QLatin1String("text"));
     QCOMPARE(binding->value().toString(), QLatin1String("Hello"));
-
-    delete rect;
 }
 
 void tst_qqmlbinding::whenAfterValue()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("test-binding2.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect {qobject_cast<QQuickRectangle*>(c.create())};
 
     QVERIFY(rect != nullptr);
     QCOMPARE(rect->color(), QColor("yellow"));
@@ -104,15 +103,13 @@ void tst_qqmlbinding::whenAfterValue()
 
     rect->setProperty("changeColor", true);
     QCOMPARE(rect->color(), QColor("red"));
-
-    delete rect;
 }
 
 void tst_qqmlbinding::restoreBinding()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("restoreBinding.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect { qobject_cast<QQuickRectangle*>(c.create()) };
     QVERIFY(rect != nullptr);
 
     QQuickRectangle *myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
@@ -134,8 +131,6 @@ void tst_qqmlbinding::restoreBinding()
     //original binding restored
     myItem->setY(49);
     QCOMPARE(myItem->x(), qreal(100-49));
-
-    delete rect;
 }
 
 void tst_qqmlbinding::restoreBindingValue()
@@ -214,7 +209,7 @@ void tst_qqmlbinding::restoreBindingWithLoop()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("restoreBindingWithLoop.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect {qobject_cast<QQuickRectangle*>(c.create())};
     QVERIFY(rect != nullptr);
 
     QQuickRectangle *myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
@@ -242,15 +237,13 @@ void tst_qqmlbinding::restoreBindingWithLoop()
 
     myItem->setY(49);
     QCOMPARE(myItem->x(), qreal(49 + 100));
-
-    delete rect;
 }
 
 void tst_qqmlbinding::restoreBindingWithoutCrash()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("restoreBindingWithoutCrash.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect {qobject_cast<QQuickRectangle*>(c.create())};
     QVERIFY(rect != nullptr);
 
     QQuickRectangle *myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
@@ -281,8 +274,6 @@ void tst_qqmlbinding::restoreBindingWithoutCrash()
     //original binding restored
     myItem->setY(49);
     QCOMPARE(myItem->x(), qreal(100-49));
-
-    delete rect;
 }
 
 //QTBUG-20692
@@ -290,15 +281,13 @@ void tst_qqmlbinding::deletedObject()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("deletedObject.qml"));
-    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QScopedPointer<QQuickRectangle> rect {qobject_cast<QQuickRectangle*>(c.create())};
     QVERIFY(rect != nullptr);
 
     QGuiApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
 
     //don't crash
     rect->setProperty("activateBinding", true);
-
-    delete rect;
 }
 
 void tst_qqmlbinding::warningOnUnknownProperty()
@@ -307,9 +296,8 @@ void tst_qqmlbinding::warningOnUnknownProperty()
 
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("unknownProperty.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item { qobject_cast<QQuickItem *>(c.create()) };
     QVERIFY(item);
-    delete item;
 
     QCOMPARE(messageHandler.messages().count(), 1);
 
@@ -323,9 +311,8 @@ void tst_qqmlbinding::warningOnReadOnlyProperty()
 
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("readonlyProperty.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item { qobject_cast<QQuickItem *>(c.create()) };
     QVERIFY(item);
-    delete item;
 
     QCOMPARE(messageHandler.messages().count(), 1);
 
@@ -339,9 +326,8 @@ void tst_qqmlbinding::disabledOnUnknownProperty()
 
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("disabledUnknown.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item { qobject_cast<QQuickItem *>(c.create()) };
     QVERIFY(item);
-    delete item;
 
     QCOMPARE(messageHandler.messages().count(), 0);
 }
@@ -352,10 +338,8 @@ void tst_qqmlbinding::disabledOnReadonlyProperty()
 
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("disabledReadonly.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item { qobject_cast<QQuickItem *>(c.create()) };
     QVERIFY(item);
-    delete item;
-
     QCOMPARE(messageHandler.messages().count(), 0);
 }
 
@@ -363,21 +347,19 @@ void tst_qqmlbinding::delayed()
 {
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("delayed.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item {qobject_cast<QQuickItem*>(c.create())};
 
     QVERIFY(item != nullptr);
     // update on creation
     QCOMPARE(item->property("changeCount").toInt(), 1);
 
-    QMetaObject::invokeMethod(item, "updateText");
+    QMetaObject::invokeMethod(item.get(), "updateText");
     // doesn't update immediately
     QCOMPARE(item->property("changeCount").toInt(), 1);
 
     QCoreApplication::processEvents();
     // only updates once (non-delayed would update twice)
     QCOMPARE(item->property("changeCount").toInt(), 2);
-
-    delete item;
 }
 
 void tst_qqmlbinding::bindingOverwriting()
@@ -387,9 +369,8 @@ void tst_qqmlbinding::bindingOverwriting()
 
     QQmlEngine engine;
     QQmlComponent c(&engine, testFileUrl("bindingOverwriting.qml"));
-    QQuickItem *item = qobject_cast<QQuickItem*>(c.create());
+    QScopedPointer<QQuickItem> item {qobject_cast<QQuickItem*>(c.create())};
     QVERIFY(item);
-    delete item;
 
     QLoggingCategory::setFilterRules(QString());
     QCOMPARE(messageHandler.messages().count(), 2);
@@ -402,6 +383,21 @@ void tst_qqmlbinding::bindToQmlComponent()
     QVERIFY(c.create());
 }
 
+// QTBUG-78943
+void tst_qqmlbinding::bindingDoesNoWeirdConversion()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("noUnexpectedStringConversion.qml"));
+    QScopedPointer<QObject> o {c.create()};
+    QVERIFY(o);
+    QObject *colorRect = o->findChild<QObject*>("colorRect");
+    QVERIFY(colorRect);
+    QCOMPARE(qvariant_cast<QColor>(colorRect->property("color")), QColorConstants::Red);
+    QObject *colorLabel = o->findChild<QObject*>("colorLabel");
+    QCOMPARE(colorLabel->property("text").toString(), QLatin1String("red"));
+    QVERIFY(colorLabel);
+}
+
 QTEST_MAIN(tst_qqmlbinding)
 
 #include "tst_qqmlbinding.moc"