From a27fd584df0ec3d1d072501d8fefe576a97a2e53 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Wed, 14 Mar 2012 13:39:31 +0100 Subject: [PATCH] Let V8 throw the exception when QML property lookup fails Only V8 knows whether a failed property lookup should actually cause a ReferenceError to be thrown. When evaluating a "typeof" expression, for example, a ReferenceError should not be thrown even if the expression involves global variables that don't exist, according to the ECMA-262 specification. QML should try to match the standard JavaScript behavior. This is achieved by simply returning an empty value handle (to signify the absence of the property), and leaving it to V8 to throw an exception as appropriate. Task-number: QTBUG-21864 Task-number: QTBUG-24448 Change-Id: I9945adcab98fc3b801371163367473d6af0ab31a Reviewed-by: Aaron Kennedy --- src/qml/qml/v8/qv8contextwrapper.cpp | 18 ++++++------------ tests/auto/qml/qqmlecmascript/data/qtbug_24448.js | 6 ++++++ tests/auto/qml/qqmlecmascript/data/qtbug_24448.qml | 7 +++++++ .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 17 +++++++++-------- 4 files changed, 28 insertions(+), 20 deletions(-) create mode 100644 tests/auto/qml/qqmlecmascript/data/qtbug_24448.js create mode 100644 tests/auto/qml/qqmlecmascript/data/qtbug_24448.qml diff --git a/src/qml/qml/v8/qv8contextwrapper.cpp b/src/qml/qml/v8/qv8contextwrapper.cpp index e7c26fd..39392b8 100644 --- a/src/qml/qml/v8/qv8contextwrapper.cpp +++ b/src/qml/qml/v8/qv8contextwrapper.cpp @@ -238,16 +238,11 @@ QQmlContextData *QV8ContextWrapper::context(v8::Handle value) return r?r->getContext():0; } -v8::Handle QV8ContextWrapper::NullGetter(v8::Local property, - const v8::AccessorInfo &info) +v8::Handle QV8ContextWrapper::NullGetter(v8::Local, + const v8::AccessorInfo &) { - QV8ContextResource *resource = v8_resource_check(info.This()); - - QV8Engine *engine = resource->engine; - - QString error = engine->toString(property) + QLatin1String(" is not defined"); - v8::ThrowException(v8::Exception::ReferenceError(engine->toString(error))); - return v8::Undefined(); + // V8 will throw a ReferenceError if appropriate ("typeof" should not throw) + return v8::Handle(); } v8::Handle QV8ContextWrapper::Getter(v8::Local property, @@ -365,9 +360,8 @@ v8::Handle QV8ContextWrapper::Getter(v8::Local property, expressionContext->unresolvedNames = true; - QString error = engine->toString(property) + QLatin1String(" is not defined"); - v8::ThrowException(v8::Exception::ReferenceError(engine->toString(error))); - return v8::Undefined(); + // V8 will throw a ReferenceError if appropriate ("typeof" should not throw) + return v8::Handle(); } v8::Handle QV8ContextWrapper::NullSetter(v8::Local property, diff --git a/tests/auto/qml/qqmlecmascript/data/qtbug_24448.js b/tests/auto/qml/qqmlecmascript/data/qtbug_24448.js new file mode 100644 index 0000000..eefe003 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/qtbug_24448.js @@ -0,0 +1,6 @@ +var test = false; +try { + eval(foo); +} catch (e) { + test = (typeof foo) === "undefined"; +} diff --git a/tests/auto/qml/qqmlecmascript/data/qtbug_24448.qml b/tests/auto/qml/qqmlecmascript/data/qtbug_24448.qml new file mode 100644 index 0000000..d8d305d --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/qtbug_24448.qml @@ -0,0 +1,7 @@ +import "qtbug_24448.js" as Test +import QtQuick 2.0 + +QtObject { + property bool test: Test.test +} + diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 3fe0a2b..8fea635 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -207,6 +207,7 @@ private slots: void deleteLater(); void in(); void typeOf(); + void qtbug_24448(); void sharedAttachedObject(); void objectName(); void writeRemovesBinding(); @@ -5376,17 +5377,9 @@ void tst_qqmlecmascript::typeOf() { QQmlComponent component(&engine, testFileUrl("typeOf.qml")); - // These warnings should not happen once QTBUG-21864 is fixed - QString warning1 = component.url().toString() + QLatin1String(":16: Error: Cannot assign [undefined] to QString"); - QString warning2 = component.url().resolved(QUrl("typeOf.js")).toString() + QLatin1String(":1: ReferenceError: a is not defined"); - - QTest::ignoreMessage(QtWarningMsg, qPrintable(warning1)); - QTest::ignoreMessage(QtWarningMsg, qPrintable(warning2)); - QObject *o = component.create(); QVERIFY(o != 0); - QEXPECT_FAIL("", "QTBUG-21864", Abort); QCOMPARE(o->property("test1").toString(), QLatin1String("undefined")); QCOMPARE(o->property("test2").toString(), QLatin1String("object")); QCOMPARE(o->property("test3").toString(), QLatin1String("number")); @@ -5400,6 +5393,14 @@ void tst_qqmlecmascript::typeOf() delete o; } +void tst_qqmlecmascript::qtbug_24448() +{ + QQmlComponent component(&engine, testFileUrl("qtbug_24448.qml")); + QScopedPointer o(component.create()); + QVERIFY(o != 0); + QVERIFY(o->property("test").toBool()); +} + void tst_qqmlecmascript::sharedAttachedObject() { QQmlComponent component(&engine, testFileUrl("sharedAttachedObject.qml")); -- 1.7.2.5