From d96d89c63c28f81f9c17666ed66222f523571f03 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 15 Mar 2012 10:15:53 +0100 Subject: [PATCH] Make QVariant conversion for JS null type symmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you pass (void*)0 to QJSEngine::toScriptValue(), or you pass a QVariant of type QMetaType::VoidStar containing a 0 value, you get back a QJSValue of type null (isNull() returns true); that's fine. However, if you called QJSValue::toVariant() on a JS null value, you would get back an invalid QVariant. The expected result is a QVariant of type QMetaType::VoidStar containing a 0 value. This makes the conversion of the JS null type symmetric and avoids loss of data. Change-Id: Ifa6e788152118f80adf9c2d7be1283f053b44294 Reviewed-by: Jędrzej Nowacki Reviewed-by: Simon Hausmann --- src/qml/qml/v8/qjsvalue.cpp | 2 +- src/qml/qml/v8/qjsvalue_impl_p.h | 2 +- src/qml/qml/v8/qv8engine.cpp | 7 +++++-- tests/auto/qml/qjsengine/tst_qjsengine.cpp | 4 ++++ tests/auto/qml/qjsvalue/tst_qjsvalue.cpp | 12 ++++++++++-- 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/qml/qml/v8/qjsvalue.cpp b/src/qml/qml/v8/qjsvalue.cpp index 4471e68a61..6ecb97d2e5 100644 --- a/src/qml/qml/v8/qjsvalue.cpp +++ b/src/qml/qml/v8/qjsvalue.cpp @@ -428,7 +428,7 @@ quint32 QJSValue::toUInt() const \table \header \li Input Type \li Result \row \li Undefined \li An invalid QVariant. - \row \li Null \li An invalid QVariant. + \row \li Null \li A QVariant containing a null pointer (QMetaType::VoidStar). \row \li Boolean \li A QVariant containing the value of the boolean. \row \li Number \li A QVariant containing the value of the number. \row \li String \li A QVariant containing the value of the string. diff --git a/src/qml/qml/v8/qjsvalue_impl_p.h b/src/qml/qml/v8/qjsvalue_impl_p.h index fbddcfa5ba..8d22204843 100644 --- a/src/qml/qml/v8/qjsvalue_impl_p.h +++ b/src/qml/qml/v8/qjsvalue_impl_p.h @@ -281,7 +281,7 @@ QVariant QJSValuePrivate::toVariant() const case CNumber: return QVariant(u.m_number); case CNull: - return QVariant(); + return QVariant(QMetaType::VoidStar, 0); case CUndefined: return QVariant(); case JSValue: diff --git a/src/qml/qml/v8/qv8engine.cpp b/src/qml/qml/v8/qv8engine.cpp index 8e8223fea1..4a340e2498 100644 --- a/src/qml/qml/v8/qv8engine.cpp +++ b/src/qml/qml/v8/qv8engine.cpp @@ -1365,7 +1365,8 @@ v8::Handle QV8Engine::variantToJS(const QVariant &value) } // Converts a JS value to a QVariant. -// Null, Undefined -> QVariant() (invalid) +// Undefined -> QVariant() (invalid) +// Null -> QVariant((void*)0) // Boolean -> QVariant(bool) // Number -> QVariant(double) // String -> QVariant(QString) @@ -1376,8 +1377,10 @@ v8::Handle QV8Engine::variantToJS(const QVariant &value) QVariant QV8Engine::variantFromJS(v8::Handle value) { Q_ASSERT(!value.IsEmpty()); - if (value->IsNull() || value->IsUndefined()) + if (value->IsUndefined()) return QVariant(); + if (value->IsNull()) + return QVariant(QMetaType::VoidStar, 0); if (value->IsBoolean()) return value->ToBoolean()->Value(); if (value->IsInt32()) diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp index 6f9cc93757..e34304b258 100644 --- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp +++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp @@ -2369,6 +2369,8 @@ void tst_QJSEngine::valueConversion_basic() QCOMPARE(eng.fromScriptValue(code), c); QCOMPARE(eng.fromScriptValue(eng.toScriptValue(c)), c); } + + QVERIFY(eng.toScriptValue(static_cast(0)).isNull()); } #if 0 // FIXME: No API for custom types @@ -2588,6 +2590,8 @@ void tst_QJSEngine::valueConversion_QVariant() } QCOMPARE(qjsvalue_cast(QJSValue(123)), QVariant(123)); + + QVERIFY(eng.toScriptValue(QVariant(QMetaType::VoidStar, 0)).isNull()); } #if 0 // FIXME: No support for custom types diff --git a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp index ad655217ad..3522f22ca2 100644 --- a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp +++ b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp @@ -993,8 +993,8 @@ void tst_QJSValue::toVariant() QCOMPARE(qjsvalue_cast(undefined), QVariant()); QJSValue null = eng.evaluate("null"); - QCOMPARE(null.toVariant(), QVariant()); - QCOMPARE(qjsvalue_cast(null), QVariant()); + QCOMPARE(null.toVariant(), QVariant(QMetaType::VoidStar, 0)); + QCOMPARE(qjsvalue_cast(null), QVariant(QMetaType::VoidStar, 0)); { QJSValue number = eng.toScriptValue(123.0); @@ -1064,6 +1064,14 @@ void tst_QJSValue::toVariant() QJSValue str = QJSValue(QString("ciao")); QCOMPARE(str.toVariant(), QVariant(QString("ciao"))); QCOMPARE(qjsvalue_cast(str), QVariant(QString("ciao"))); + + QJSValue undef = QJSValue(QJSValue::UndefinedValue); + QCOMPARE(undef.toVariant(), QVariant()); + QCOMPARE(qjsvalue_cast(undef), QVariant()); + + QJSValue nil = QJSValue(QJSValue::NullValue); + QCOMPARE(nil.toVariant(), QVariant(QMetaType::VoidStar, 0)); + QCOMPARE(qjsvalue_cast(nil), QVariant(QMetaType::VoidStar, 0)); } #if 0 // FIXME: No automatic sequence conversion