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>

5 files changed, 69 insertions, 41 deletions

diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index f89608cd5d..a4270628e8 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -445,6 +445,8 @@ void QQmlObjectCreator::setPropertyValue(const QQmlPropertyData *property, const
                 QV4::ScopedString s(scope, v4->newString(stringValue));
                 _vmeMetaObject->setVMEProperty(property->coreIndex(), s);
             } else {
+                // ### Qt 6: Doing the conversion here where we don't know the eventual target type is rather strange
+                // and caused for instance QTBUG-78943
                 QVariant value = QQmlStringConverters::variantFromString(stringValue);
                 property->writeProperty(_qobject, &value, propertyWriteFlags);
             }
diff --git a/src/qml/types/qqmlbind.cpp b/src/qml/types/qqmlbind.cpp
index 8f40f4f704..36de16a818 100644
--- a/src/qml/types/qqmlbind.cpp
+++ b/src/qml/types/qqmlbind.cpp
@@ -78,7 +78,7 @@ public:
     QQmlNullableValue<bool> when;
     QPointer<QObject> obj;
     QString propName;
-    QQmlNullableValue<QVariant> value;
+    QQmlNullableValue<QJSValue> value;
     QQmlProperty prop;
     QQmlAbstractBinding::Ptr prevBind;
     QV4::PersistentValue v4Value;
@@ -293,13 +293,13 @@ void QQmlBind::setProperty(const QString &p)
     The value to be set on the target object and property.  This can be a
     constant (which isn't very useful), or a bound expression.
 */
-QVariant QQmlBind::value() const
+QJSValue QQmlBind::value() const
 {
     Q_D(const QQmlBind);
     return d->value.value;
 }
 
-void QQmlBind::setValue(const QVariant &v)
+void QQmlBind::setValue(const QJSValue &v)
 {
     Q_D(QQmlBind);
     d->value = v;
@@ -502,7 +502,7 @@ void QQmlBind::eval()
         QQmlPropertyPrivate::removeBinding(d->prop);
     }
 
-    d->prop.write(d->value.value);
+    d->prop.write(d->value.value.toVariant());
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/types/qqmlbind_p.h b/src/qml/types/qqmlbind_p.h
index 22007a3d25..ba040d2a0b 100644
--- a/src/qml/types/qqmlbind_p.h
+++ b/src/qml/types/qqmlbind_p.h
@@ -75,7 +75,7 @@ private:
     Q_INTERFACES(QQmlPropertyValueSource)
     Q_PROPERTY(QObject *target READ object WRITE setObject)
     Q_PROPERTY(QString property READ property WRITE setProperty)
-    Q_PROPERTY(QVariant value READ value WRITE setValue)
+    Q_PROPERTY(QJSValue value READ value WRITE setValue)
     Q_PROPERTY(bool when READ when WRITE setWhen)
     Q_PROPERTY(bool delayed READ delayed WRITE setDelayed REVISION 8)
     Q_PROPERTY(RestorationMode restoreMode READ restoreMode WRITE setRestoreMode
@@ -95,8 +95,8 @@ public:
     QString property() const;
     void setProperty(const QString &);
 
-    QVariant value() const;
-    void setValue(const QVariant &);
+    QJSValue value() const;
+    void setValue(const QJSValue &);
 
     bool delayed() const;
     void setDelayed(bool);
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"