Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kent Hansen authored and Qt by Nokia committed Mar 20, 2012
1 parent d38e9e4 commit d96d89c
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/qml/qml/v8/qjsvalue.cpp
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/qml/qml/v8/qjsvalue_impl_p.h
Expand Up @@ -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:
Expand Down
7 changes: 5 additions & 2 deletions src/qml/qml/v8/qv8engine.cpp
Expand Up @@ -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)
Expand All @@ -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())
Expand Down
4 changes: 4 additions & 0 deletions tests/auto/qml/qjsengine/tst_qjsengine.cpp
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d96d89c

Please sign in to comment.