From b1da5cb07922e786bd3223317651284b73159e82 Mon Sep 17 00:00:00 2001 From: Matthew Vogt Date: Thu, 19 Jan 2012 15:54:24 +1000 Subject: Assigning empty object to Q_PROPERTY(QVariantMap) Correct the evaluation of an empty javascript object during assignment to a QVariantMap property. Task-number: QTBUG-23586 Change-Id: Ifa891a017690a36bd5837bc6b4dd0e47eb515a46 Reviewed-by: Martin Jones --- src/declarative/qml/v8/qv8engine.cpp | 15 +-- .../data/assignEmptyVariantMap.qml | 5 + .../tst_qdeclarativeproperty.cpp | 114 ++++++++++++++++++++- tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp | 6 ++ 4 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeproperty/data/assignEmptyVariantMap.qml diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp index 448734638f..dbe8fbe774 100644 --- a/src/declarative/qml/v8/qv8engine.cpp +++ b/src/declarative/qml/v8/qv8engine.cpp @@ -501,10 +501,6 @@ QVariant QV8Engine::toBasicVariant(v8::Handle value) if (!value->IsFunction()) { v8::Context::Scope scope(context()); v8::Handle object = value->ToObject(); - v8::Local properties = object->GetPropertyNames(); - int length = properties->Length(); - if (length == 0) - return QVariant(); return variantMapFromJS(object); } @@ -1088,14 +1084,19 @@ v8::Local QV8Engine::variantMapToJS(const QVariantMap &vmap) QVariantMap QV8Engine::variantMapFromJS(v8::Handle jsObject) { QVariantMap result; + + v8::HandleScope handleScope; + v8::Handle propertyNames = jsObject->GetPropertyNames(); + uint32_t length = propertyNames->Length(); + if (length == 0) + return result; + int hash = jsObject->GetIdentityHash(); if (visitedConversionObjects.contains(hash)) return result; // Avoid recursion. + visitedConversionObjects.insert(hash); - v8::HandleScope handleScope; // TODO: Only object's own property names. Include non-enumerable properties. - v8::Handle propertyNames = jsObject->GetPropertyNames(); - uint32_t length = propertyNames->Length(); for (uint32_t i = 0; i < length; ++i) { v8::Handle name = propertyNames->Get(i); result.insert(QJSConverter::toString(name->ToString()), variantFromJS(jsObject->Get(name))); diff --git a/tests/auto/declarative/qdeclarativeproperty/data/assignEmptyVariantMap.qml b/tests/auto/declarative/qdeclarativeproperty/data/assignEmptyVariantMap.qml new file mode 100644 index 0000000000..a9e51c1255 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeproperty/data/assignEmptyVariantMap.qml @@ -0,0 +1,5 @@ +import QtQuick 2.0 + +Item { + Component.onCompleted: { o.variantMap = {}; } +} diff --git a/tests/auto/declarative/qdeclarativeproperty/tst_qdeclarativeproperty.cpp b/tests/auto/declarative/qdeclarativeproperty/tst_qdeclarativeproperty.cpp index 348c2582c0..5604df885f 100644 --- a/tests/auto/declarative/qdeclarativeproperty/tst_qdeclarativeproperty.cpp +++ b/tests/auto/declarative/qdeclarativeproperty/tst_qdeclarativeproperty.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -124,10 +125,14 @@ private slots: void urlHandling_data(); void urlHandling(); + void variantMapHandling_data(); + void variantMapHandling(); + // Bugs void crashOnValueProperty(); void aliasPropertyBindings(); void noContext(); + void assignEmptyVariantMap(); void copy(); private: @@ -188,6 +193,7 @@ class PropertyObject : public QObject Q_PROPERTY(QRect rectProperty READ rectProperty) Q_PROPERTY(QRect wrectProperty READ wrectProperty WRITE setWRectProperty) Q_PROPERTY(QUrl url READ url WRITE setUrl) + Q_PROPERTY(QVariantMap variantMap READ variantMap WRITE setVariantMap) Q_PROPERTY(int resettableProperty READ resettableProperty WRITE setResettableProperty RESET resetProperty) Q_PROPERTY(int propertyWithNotify READ propertyWithNotify WRITE setPropertyWithNotify NOTIFY oddlyNamedNotifySignal) Q_PROPERTY(MyQmlObject *qmlObject READ qmlObject) @@ -205,6 +211,9 @@ public: QUrl url() { return m_url; } void setUrl(const QUrl &u) { m_url = u; } + QVariantMap variantMap() const { return m_variantMap; } + void setVariantMap(const QVariantMap &variantMap) { m_variantMap = variantMap; } + int resettableProperty() const { return m_resetProperty; } void setResettableProperty(int r) { m_resetProperty = r; } void resetProperty() { m_resetProperty = 9; } @@ -213,6 +222,7 @@ public: void setPropertyWithNotify(int i) { m_propertyWithNotify = i; emit oddlyNamedNotifySignal(); } MyQmlObject *qmlObject() { return &m_qmlObject; } + signals: void clicked(); void oddlyNamedNotifySignal(); @@ -221,6 +231,7 @@ private: int m_resetProperty; QRect m_rect; QUrl m_url; + QVariantMap m_variantMap; int m_propertyWithNotify; MyQmlObject m_qmlObject; }; @@ -1196,6 +1207,32 @@ void tst_qdeclarativeproperty::write() QCOMPARE(o.url(), result); } + // VariantMap-property + QVariantMap vm; + vm.insert("key", "value"); + + { + PropertyObject o; + QDeclarativeProperty p(&o, "variantMap"); + + QCOMPARE(p.write(vm), true); + QCOMPARE(o.variantMap(), vm); + + QDeclarativeProperty p2(&o, "variantMap", engine.rootContext()); + + QCOMPARE(p2.write(vm), true); + QCOMPARE(o.variantMap(), vm); + } + { // static + PropertyObject o; + + QCOMPARE(QDeclarativeProperty::write(&o, "variantMap", vm), true); + QCOMPARE(o.variantMap(), vm); + + QCOMPARE(QDeclarativeProperty::write(&o, "variantMap", vm, engine.rootContext()), true); + QCOMPARE(o.variantMap(), vm); + } + // Attached property { QDeclarativeComponent component(&engine); @@ -1319,7 +1356,6 @@ void tst_qdeclarativeproperty::writeObjectToList() QCOMPARE(list.at(0), qobject_cast(object)); } -Q_DECLARE_METATYPE(QList); void tst_qdeclarativeproperty::writeListToList() { QDeclarativeComponent containerComponent(&engine); @@ -1446,6 +1482,59 @@ void tst_qdeclarativeproperty::urlHandling() } } +void tst_qdeclarativeproperty::variantMapHandling_data() +{ + QTest::addColumn("vm"); + + // Object literals + { + QVariantMap m; + QTest::newRow("{}") << m; + } + { + QVariantMap m; + m["a"] = QVariantMap(); + QTest::newRow("{ a:{} }") << m; + } + { + QVariantMap m, m2; + m2["b"] = 10; + m2["c"] = 20; + m["a"] = m2; + QTest::newRow("{ a:{b:10, c:20} }") << m; + } + { + QVariantMap m; + m["a"] = 10; + m["b"] = QVariantList() << 20 << 30; + QTest::newRow("{ a:10, b:[20, 30]}") << m; + } + + // Cyclic objects + { + QVariantMap m; + m["p"] = QVariantMap(); + QTest::newRow("var o={}; o.p=o") << m; + } + { + QVariantMap m; + m["p"] = 123; + m["q"] = QVariantMap(); + QTest::newRow("var o={}; o.p=123; o.q=o") << m; + } +} + +void tst_qdeclarativeproperty::variantMapHandling() +{ + QFETCH(QVariantMap, vm); + + PropertyObject o; + QDeclarativeProperty p(&o, "variantMap"); + + QCOMPARE(p.write(vm), true); + QCOMPARE(o.variantMap(), vm); +} + void tst_qdeclarativeproperty::crashOnValueProperty() { QDeclarativeEngine *engine = new QDeclarativeEngine; @@ -1596,6 +1685,29 @@ void tst_qdeclarativeproperty::noContext() delete b; } +void tst_qdeclarativeproperty::assignEmptyVariantMap() +{ + PropertyObject o; + + QVariantMap map; + map.insert("key", "value"); + o.setVariantMap(map); + QCOMPARE(o.variantMap().count(), 1); + QCOMPARE(o.variantMap().isEmpty(), false); + + QDeclarativeContext context(&engine); + context.setContextProperty("o", &o); + + QDeclarativeComponent component(&engine, testFileUrl("assignEmptyVariantMap.qml")); + QObject *obj = component.create(&context); + QVERIFY(obj); + + QCOMPARE(o.variantMap().count(), 0); + QCOMPARE(o.variantMap().isEmpty(), true); + + delete obj; +} + void tst_qdeclarativeproperty::initTestCase() { QDeclarativeDataTest::initTestCase(); diff --git a/tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp b/tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp index 5a9ccc521c..e3e259dcbc 100644 --- a/tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp +++ b/tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp @@ -4021,6 +4021,12 @@ void tst_QJSValue::nestedObjectToVariant_data() << QVariant(QVariantList() << 123 << QVariant(QVariantList() << 456 << QVariant(QVariantList()))); // Object literals + { + QVariantMap m; + QTest::newRow("{}") + << QString::fromLatin1("({})") + << QVariant(m); + } { QVariantMap m; m["a"] = QVariantMap(); -- cgit v1.2.3