diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2023-12-15 16:39:45 +0100 |
---|---|---|
committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2023-12-19 15:39:18 +0100 |
commit | 4d7ce35bd2b3a7319973303f163c26cdf67c73f6 (patch) | |
tree | 25cb9c5da99ba81ad9972a2984aef21cb63b2928 | |
parent | af27f1ce3bf737f1b3e3d0413f1ff02cc5e32790 (diff) |
QJSValue: convert more aggressively to QVariant
Normally, we don't want to convert aggressively between JS objects and
QVariant, as that is prone to losing information. However,
QJSValue::toVariant is documented to attempt lossy conversions. Restore
the behavior of Qt < 6.5.3 for it. This is done by replacing the boolean
indicating we should wrap JS objects into QJSValue with an enum instead.
That enum introduces a third state ("Aggressive"), which is only used
for QJSValue::toVariant. All other users of QJSEngine::toVariant behave
as before (post 6.5.3).
Function objects are still not converted, as we know that this would be
a futile attempt, and more importantly, to keep the behavior that
existed before Qt 6.5.3.
Amends 43077556550c6b17226a7d393ec844b605c9c678 which introduced the
regression and afe96c4d633146df477012975824b8ab65034239 which fixed the
issue only partially.
Pick-to: 6.5 6.6 6.7
Fixes: QTBUG-119963
Change-Id: I07d9901437812579ac5b873a4dff4de60c8f617e
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
-rw-r--r-- | src/qml/jsapi/qjsvalue.cpp | 7 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4engine.cpp | 48 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4engine_p.h | 1 | ||||
-rw-r--r-- | tests/auto/qml/qjsvalue/tst_qjsvalue.cpp | 19 |
4 files changed, 57 insertions, 18 deletions
diff --git a/src/qml/jsapi/qjsvalue.cpp b/src/qml/jsapi/qjsvalue.cpp index bc75526e8d..f622c3fb23 100644 --- a/src/qml/jsapi/qjsvalue.cpp +++ b/src/qml/jsapi/qjsvalue.cpp @@ -638,8 +638,11 @@ QVariant QJSValue::toVariant(QJSValue::ObjectConversionBehavior behavior) const if (val.isString()) return QVariant(val.toQString()); if (val.as<QV4::Managed>()) { - return QV4::ExecutionEngine::toVariant( - val, /*typeHint*/ QMetaType{}, behavior == RetainJSObjects); + if (behavior == RetainJSObjects) + return QV4::ExecutionEngine::toVariant( + val, /*typeHint*/ QMetaType{}, /*createJSValueForObjectsAndSymbols=*/ true); + else + return QV4::ExecutionEngine::toVariantLossy(val); } Q_ASSERT(false); diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index f28c5ef163..2d128e05dc 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -1479,11 +1479,13 @@ QQmlError ExecutionEngine::catchExceptionAsQmlError() // Variant conversion code typedef QSet<QV4::Heap::Object *> V4ObjectSet; +enum class JSToQVariantConversionBehavior {Never, Safish, Aggressive }; static QVariant toVariant( - const QV4::Value &value, QMetaType typeHint, bool createJSValueForObjectsAndSymbols, + const QV4::Value &value, QMetaType typeHint, JSToQVariantConversionBehavior conversionBehavior, V4ObjectSet *visitedObjects); static QObject *qtObjectFromJS(const QV4::Value &value); -static QVariant objectToVariant(const QV4::Object *o, V4ObjectSet *visitedObjects = nullptr); +static QVariant objectToVariant(const QV4::Object *o, V4ObjectSet *visitedObjects = nullptr, + JSToQVariantConversionBehavior behavior = JSToQVariantConversionBehavior::Safish); static bool convertToNativeQObject(const QV4::Value &value, QMetaType targetType, void **result); static QV4::ReturnedValue variantMapToJS(QV4::ExecutionEngine *v4, const QVariantMap &vmap); static QV4::ReturnedValue variantToJS(QV4::ExecutionEngine *v4, const QVariant &value) @@ -1491,8 +1493,7 @@ static QV4::ReturnedValue variantToJS(QV4::ExecutionEngine *v4, const QVariant & return v4->metaTypeToJS(value.metaType(), value.constData()); } -static QVariant toVariant( - const QV4::Value &value, QMetaType metaType, bool createJSValueForObjectsAndSymbols, +static QVariant toVariant(const QV4::Value &value, QMetaType metaType, JSToQVariantConversionBehavior conversionBehavior, V4ObjectSet *visitedObjects) { Q_ASSERT (!value.isEmpty()); @@ -1589,7 +1590,7 @@ static QVariant toVariant( } } - asVariant = toVariant(arrayValue, valueMetaType, false, visitedObjects); + asVariant = toVariant(arrayValue, valueMetaType, JSToQVariantConversionBehavior::Never, visitedObjects); if (valueMetaType == QMetaType::fromType<QVariant>()) { retnAsIterable.metaContainer().addValue(retn.data(), &asVariant); } else { @@ -1654,7 +1655,7 @@ static QVariant toVariant( if (const ArrayBuffer *d = value.as<ArrayBuffer>()) return d->asByteArray(); if (const Symbol *symbol = value.as<Symbol>()) { - return createJSValueForObjectsAndSymbols + return conversionBehavior == JSToQVariantConversionBehavior::Never ? QVariant::fromValue(QJSValuePrivate::fromReturnedValue(symbol->asReturnedValue())) : symbol->descriptiveString(); } @@ -1675,20 +1676,27 @@ static QVariant toVariant( return result; } - if (createJSValueForObjectsAndSymbols) + if (conversionBehavior == JSToQVariantConversionBehavior::Never) return QVariant::fromValue(QJSValuePrivate::fromReturnedValue(o->asReturnedValue())); - return objectToVariant(o, visitedObjects); + return objectToVariant(o, visitedObjects, conversionBehavior); } +QVariant ExecutionEngine::toVariantLossy(const Value &value) +{ + return ::toVariant(value, QMetaType(), JSToQVariantConversionBehavior::Aggressive, nullptr); +} QVariant ExecutionEngine::toVariant( const Value &value, QMetaType typeHint, bool createJSValueForObjectsAndSymbols) { - return ::toVariant(value, typeHint, createJSValueForObjectsAndSymbols, nullptr); + auto behavior = createJSValueForObjectsAndSymbols ? JSToQVariantConversionBehavior::Never + : JSToQVariantConversionBehavior::Safish; + return ::toVariant(value, typeHint, behavior, nullptr); } -static QVariantMap objectToVariantMap(const QV4::Object *o, V4ObjectSet *visitedObjects) +static QVariantMap objectToVariantMap(const QV4::Object *o, V4ObjectSet *visitedObjects, + JSToQVariantConversionBehavior conversionBehvior) { QVariantMap map; QV4::Scope scope(o->engine()); @@ -1703,12 +1711,13 @@ static QVariantMap objectToVariantMap(const QV4::Object *o, V4ObjectSet *visited QString key = name->toQStringNoThrow(); map.insert(key, ::toVariant( val, /*type hint*/ QMetaType {}, - /*createJSValueForObjectsAndSymbols*/false, visitedObjects)); + conversionBehvior, visitedObjects)); } return map; } -static QVariant objectToVariant(const QV4::Object *o, V4ObjectSet *visitedObjects) +static QVariant objectToVariant(const QV4::Object *o, V4ObjectSet *visitedObjects, + JSToQVariantConversionBehavior conversionBehvior) { Q_ASSERT(o); @@ -1736,13 +1745,20 @@ static QVariant objectToVariant(const QV4::Object *o, V4ObjectSet *visitedObject int length = a->getLength(); for (int ii = 0; ii < length; ++ii) { v = a->get(ii); - list << ::toVariant(v, QMetaType {}, /*createJSValueForObjectsAndSymbols*/false, + list << ::toVariant(v, QMetaType {}, conversionBehvior, visitedObjects); } result = list; - } else if (o->getPrototypeOf() == o->engine()->objectPrototype()->d()) { - result = objectToVariantMap(o, visitedObjects); + } else if (o->getPrototypeOf() == o->engine()->objectPrototype()->d() + || (conversionBehvior == JSToQVariantConversionBehavior::Aggressive && + !o->as<QV4::FunctionObject>())) { + /* FunctionObject is excluded for historical reasons, even though + objects with a custom prototype risk losing information + But the Aggressive path is used only in QJSValue::toVariant + which is documented to be lossy + */ + result = objectToVariantMap(o, visitedObjects, conversionBehvior); } else { // If it's not a plain object, we can only save it as QJSValue. result = QVariant::fromValue(QJSValuePrivate::fromReturnedValue(o->asReturnedValue())); @@ -1968,7 +1984,7 @@ QVariantMap ExecutionEngine::variantMapFromJS(const Object *o) Q_ASSERT(o); V4ObjectSet visitedObjects; visitedObjects.insert(o->d()); - return objectToVariantMap(o, &visitedObjects); + return objectToVariantMap(o, &visitedObjects, JSToQVariantConversionBehavior::Safish); } // Converts a QVariantMap to JS. diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h index 3bbb0d9646..8ab3d62311 100644 --- a/src/qml/jsruntime/qv4engine_p.h +++ b/src/qml/jsruntime/qv4engine_p.h @@ -665,6 +665,7 @@ public: // variant conversions static QVariant toVariant( const QV4::Value &value, QMetaType typeHint, bool createJSValueForObjectsAndSymbols = true); + static QVariant toVariantLossy(const QV4::Value &value); QV4::ReturnedValue fromVariant(const QVariant &); QV4::ReturnedValue fromVariant( const QVariant &variant, Heap::Object *parent, int property, uint flags); diff --git a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp index ada191d863..6c7612b77e 100644 --- a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp +++ b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp @@ -1113,6 +1113,25 @@ void tst_QJSValue::toVariant() QCOMPARE(func.toVariant().metaType(), QMetaType::fromType<QJSValue>()); } + + // object with custom prototype + { + QJSValue object = eng.evaluate(R"js( + (function(){ + function Person(firstName, lastName) { + this.firstName = firstName; + this.lastName = lastName; + } + return new Person("John", "Doe"); + })(); + )js"); + QVERIFY(object.isObject()); + auto asVariant = object.toVariant(); + QCOMPARE(asVariant.metaType(), QMetaType::fromType<QVariantMap>()); + auto variantMap = asVariant.value<QVariantMap>(); + QVERIFY(variantMap.contains("firstName")); + QCOMPARE(variantMap["firstName"].toString(), "John"); + } } void tst_QJSValue::toPrimitive_data() |