Make QVariant conversion for JS null type symmetric

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 <jedrzej.nowacki@nokia.com>
Reviewed-by: Simon Hausmann <simon.hausmann@nokia.com>

diff --git a/src/qml/qml/v8/qjsvalue.cpp b/src/qml/qml/v8/qjsvalue.cpp
index 4471e68..6ecb97d 100644 (file)
--- 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 fbddcfa..8d22204 100644 (file)
--- 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 8e8223f..4a340e2 100644 (file)
--- a/src/qml/qml/v8/qv8engine.cpp
+++ b/src/qml/qml/v8/qv8engine.cpp
@@ -1365,7 +1365,8 @@ v8::Handle<v8::Value> 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<v8::Value> QV8Engine::variantToJS(const QVariant &value)
 QVariant QV8Engine::variantFromJS(v8::Handle<v8::Value> 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 6f9cc93..e34304b 100644 (file)
--- 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<QChar>(code), c);
         QCOMPARE(eng.fromScriptValue<QChar>(eng.toScriptValue(c)), c);
     }
+
+    QVERIFY(eng.toScriptValue(static_cast<void *>(0)).isNull());
 }
 
 #if 0 // FIXME: No API for custom types
@@ -2588,6 +2590,8 @@ void tst_QJSEngine::valueConversion_QVariant()
     }
 
     QCOMPARE(qjsvalue_cast<QVariant>(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 ad65521..3522f22 100644 (file)
--- 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<QVariant>(undefined), QVariant());
 
     QJSValue null = eng.evaluate("null");
-    QCOMPARE(null.toVariant(), QVariant());
-    QCOMPARE(qjsvalue_cast<QVariant>(null), QVariant());
+    QCOMPARE(null.toVariant(), QVariant(QMetaType::VoidStar, 0));
+    QCOMPARE(qjsvalue_cast<QVariant>(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<QVariant>(str), QVariant(QString("ciao")));
+
+        QJSValue undef = QJSValue(QJSValue::UndefinedValue);
+        QCOMPARE(undef.toVariant(), QVariant());
+        QCOMPARE(qjsvalue_cast<QVariant>(undef), QVariant());
+
+        QJSValue nil = QJSValue(QJSValue::NullValue);
+        QCOMPARE(nil.toVariant(), QVariant(QMetaType::VoidStar, 0));
+        QCOMPARE(qjsvalue_cast<QVariant>(nil), QVariant(QMetaType::VoidStar, 0));
     }
 
 #if 0 // FIXME: No automatic sequence conversion