Bug 1259822 - Show property key in the error message when target object value is null or undefined. r=jorendorff

dom/browser-element/mochitest/browserElement_ExecuteScript.js

@@ -91,7 +91,7 @@ function runTest() {
       ok(c(rv, 'YQ=='), `scriptId: ${scriptId++}`);
       return iframe.executeScript('window.wrappedJSObject.btoa("a")', {url})
     }, bail).then(bail, (error) => {
-      is(error.message, 'TypeError: window.wrappedJSObject is undefined', `scriptId: ${scriptId++}`);
+      is(error.message, `TypeError: window.wrappedJSObject is undefined, can't access property "btoa" of it`, `scriptId: ${scriptId++}`);
       return iframe.executeScript('42', {})
     }).then(bail, error => {
       is(error.name, 'InvalidAccessError', `scriptId: ${scriptId++}`);

dom/u2f/tests/frame_appid_facet_insecure.html

@@ -33,7 +33,7 @@ async function doTests() {
       local_ok(false, "Callbacks should not be called.");
     });
   } catch (err) {
-    local_ok(err == "TypeError: window.u2f is undefined", "accessing window.u2f should have thrown from an insecure origin");
+    local_ok(err.toString().includes("TypeError: window.u2f is undefined"), "accessing window.u2f should have thrown from an insecure origin");
   }
 
   try {

js/src/jit-test/tests/basic/bug827104.js

@@ -10,4 +10,4 @@ var e;
 try {
     f();
 } catch (error) {e = error;}
-assertEq(e.toString(), 'TypeError: a[i] is undefined');
+assertEq(e.toString(), `TypeError: a[i] is undefined, can't access property 0 of it`);

js/src/jit-test/tests/basic/expression-autopsy.js

@@ -18,7 +18,7 @@ function check_one(expected, f, err) {
 ieval = eval;
 function check(expr, expected=expr, testStrict=true) {
     var end, err;
-    for ([end, err] of [[".random_prop", " is undefined"], ["()", " is not a function"]]) {
+    for ([end, err] of [[".random_prop", ` is undefined, can't access property \"random_prop" of it`], ["()", " is not a function"]]) {
         var statement = "o = {};" + expr + end, f;
         var cases = [
             // Global scope
@@ -102,7 +102,7 @@ check_one("6", (function () { 6() }), " is not a function");
 check_one("4", (function() { (4||eval)(); }), " is not a function");
 check_one("0", (function () { Array.prototype.reverse.call('123'); }), " is read-only");
 check_one("[...][Symbol.iterator](...).next(...).value",
-          function () { ieval("{ let x; var [a, b, [c0, c1]] = [x, x, x]; }") }, " is undefined");
+          function () { ieval("{ let x; var [a, b, [c0, c1]] = [x, x, x]; }") }, " is undefined, can't access property Symbol.iterator of it");
 check_one("(void 1)", function() { (void 1)(); }, " is not a function");
 check_one("(void o[1])", function() { var o = []; (void o[1])() }, " is not a function");
 

js/src/jit-test/tests/basic/iterable-error-messages.js

@@ -15,7 +15,7 @@ function testForOf(val) {
 for (v of [{}, Math, new Proxy({}, {})]) {
     assertThrowsMsg(() => testForOf(v), "val is not iterable");
 }
-assertThrowsMsg(() => testForOf(null), "val is null");
+assertThrowsMsg(() => testForOf(null), "val is null, can't access property Symbol.iterator of it");
 assertThrowsMsg(() => { for (var x of () => 1) {}}, "() => 1 is not iterable");
 
 // Destructuring
@@ -25,7 +25,7 @@ function testDestr(val) {
 for (v of [{}, Math, new Proxy({}, {})]) {
     assertThrowsMsg(() => testDestr(v), "val is not iterable");
 }
-assertThrowsMsg(() => testDestr(null), "val is null");
+assertThrowsMsg(() => testDestr(null), "val is null, can't access property Symbol.iterator of it");
 assertThrowsMsg(() => { [a, b] = () => 1; }, "() => 1 is not iterable");
 
 // Spread
@@ -35,5 +35,5 @@ function testSpread(val) {
 for (v of [{}, Math, new Proxy({}, {})]) {
     assertThrowsMsg(() => testSpread(v), "val is not iterable");
 }
-assertThrowsMsg(() => testSpread(null), "val is null");
+assertThrowsMsg(() => testSpread(null), "val is null, can't access property Symbol.iterator of it");
 assertThrowsMsg(() => { [...() => 1]; }, "() => 1 is not iterable");

js/src/jit-test/tests/basic/testBug604210.js

@@ -6,6 +6,6 @@ function f() {
     } catch (e) {
         msg = '' + e;
     }
-    assertEq(msg, "TypeError: x is undefined");
+    assertEq(msg, `TypeError: x is undefined, can't access property "foo" of it`);
 }
 f();

js/src/jit-test/tests/debug/bug1275001.js

@@ -17,7 +17,7 @@ function check_one(expected, f, err) {
 ieval = eval
 function check(expr, expected = expr) {
     var end, err
-    for ([end, err] of[[".random_prop", " is undefined" ]]) 
+    for ([end, err] of[[".random_prop", ` is undefined, can't access property \"random_prop" of it` ]]) 
          statement = "o = {};" + expr + end;
          cases = [
             function() { return ieval("var undef;" + statement); },

js/src/jit-test/tests/ion/bug913749.js

@@ -15,7 +15,7 @@ for (var i = 0; i < 3; i++) {
 	x.toString();
 	assertEq(0, 1);
     } catch (e) {
-	assertEq(e.message === "y is undefined" ||
+	assertEq(e.message === `y is undefined, can't access property "length" of it` ||
 		 e.message === "undefined has no properties", true);
     }
 }

js/src/jit/BaselineIC.cpp

@@ -1880,7 +1880,7 @@ DoSetElemFallback(JSContext* cx, BaselineFrame* frame, ICSetElem_Fallback* stub_
                op == JSOP_INITELEM_ARRAY ||
                op == JSOP_INITELEM_INC);
 
-    RootedObject obj(cx, ToObjectFromStack(cx, objv));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, objv, index));
     if (!obj)
         return false;
 
@@ -2769,7 +2769,7 @@ DoSetPropFallback(JSContext* cx, BaselineFrame* frame, ICSetProp_Fallback* stub_
         name = script->getName(pc);
     RootedId id(cx, NameToId(name));
 
-    RootedObject obj(cx, ToObjectFromStack(cx, lhs));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, lhs, id));
     if (!obj)
         return false;
     RootedShape oldShape(cx, obj->maybeShape());

js/src/js.msg

@@ -54,6 +54,8 @@ MSG_DEF(JSMSG_CANT_CONVERT_TO,         2, JSEXN_TYPEERR, "can't convert {0} to {
 MSG_DEF(JSMSG_TOPRIMITIVE_NOT_CALLABLE, 2, JSEXN_TYPEERR, "can't convert {0} to {1}: its [Symbol.toPrimitive] property is not a function")
 MSG_DEF(JSMSG_TOPRIMITIVE_RETURNED_OBJECT, 2, JSEXN_TYPEERR, "can't convert {0} to {1}: its [Symbol.toPrimitive] method returned an object")
 MSG_DEF(JSMSG_NO_PROPERTIES,           1, JSEXN_TYPEERR, "{0} has no properties")
+MSG_DEF(JSMSG_PROPERTY_FAIL,           2, JSEXN_TYPEERR, "can't access property {0} of {1}")
+MSG_DEF(JSMSG_PROPERTY_FAIL_EXPR,      3, JSEXN_TYPEERR, "{0} is {1}, can't access property {2} of it")
 MSG_DEF(JSMSG_BAD_REGEXP_FLAG,         1, JSEXN_SYNTAXERR, "invalid regular expression flag {0}")
 MSG_DEF(JSMSG_INVALID_DATA_VIEW_LENGTH, 0, JSEXN_RANGEERR, "invalid data view length")
 MSG_DEF(JSMSG_OFFSET_LARGER_THAN_FILESIZE, 0, JSEXN_RANGEERR, "offset is larger than filesize")

js/src/jsapi-tests/testErrorInterceptor.cpp

@@ -52,7 +52,7 @@ BEGIN_TEST(testErrorInterceptor)
         "ReferenceError: I am a ReferenceError\0",
         "SyntaxError: I am a SyntaxError\0",
         "5\0",
-        "TypeError: undefined has no properties\0",
+        "TypeError: can't access property 0 of undefined\0",
         "ReferenceError: foo is not defined\0",
         "SyntaxError: expected expression, got end of script\0",
     };

js/src/tests/non262/extensions/regress-353116.js

@@ -19,7 +19,7 @@ function test()
   printBugNumber(BUGNUMBER);
   printStatus (summary);
 
-  expect = 'TypeError: undefined has no properties';
+  expect = `TypeError: can't access property "y" of undefined`;
   actual = 'No Error';
 
   try
@@ -32,7 +32,7 @@ function test()
   }
   reportCompare(expect, actual, summary);
 
-  expect = 'TypeError: null has no properties';
+  expect = `TypeError: can't access property "y" of null`;
   actual = 'No Error';
 
   try
@@ -45,7 +45,7 @@ function test()
   }
   reportCompare(expect, actual, summary);
 
-  expect = 'TypeError: x is undefined';
+  expect = `TypeError: x is undefined, can't access property "y" of it`;
   actual = 'No Error';
 
   try
@@ -59,7 +59,7 @@ function test()
   }
   reportCompare(expect, actual, summary);
 
-  expect = 'TypeError: x is null';
+  expect = `TypeError: x is null, can't access property "y" of it`;
   actual = 'No Error';
 
   try

js/src/tests/non262/regress/regress-469625-03.js

@@ -25,7 +25,7 @@ function test()
     var [a, b, [c0, c1]] = [x, x, x];
   }
 
-  expect = `TypeError: [...][Symbol.iterator](...).next(...).value is null`;
+  expect = `TypeError: [...][Symbol.iterator](...).next(...).value is null, can't access property Symbol.iterator of it`;
   actual = 'No Error';
   try
   {

js/src/tests/non262/regress/regress-469758.js

@@ -9,6 +9,6 @@ try {
     err = e;
 }
 assertEq(err instanceof TypeError, true);
-assertEq(err.message, "[][j] is undefined");
+assertEq(err.message, "[][j] is undefined, can't access property 2 of it");
 
 reportCompare(0, 0, 'ok');

js/src/vm/Interpreter-inl.h

@@ -538,7 +538,7 @@ GetPrimitiveElementOperation(JSContext* cx, JSOp op, JS::HandleValue receiver,
     MOZ_ASSERT(op == JSOP_GETELEM || op == JSOP_CALLELEM);
 
     // FIXME: Bug 1234324 We shouldn't be boxing here.
-    RootedObject boxed(cx, ToObjectFromStack(cx, receiver));
+    RootedObject boxed(cx, ToObjectFromStackForPropertyAccess(cx, receiver, key));
     if (!boxed)
         return false;
 

js/src/vm/Interpreter.cpp

@@ -263,7 +263,7 @@ SetPropertyOperation(JSContext* cx, JSOp op, HandleValue lval, HandleId id, Hand
 {
     MOZ_ASSERT(op == JSOP_SETPROP || op == JSOP_STRICTSETPROP);
 
-    RootedObject obj(cx, ToObjectFromStack(cx, lval));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, lval, id));
     if (!obj)
         return false;
 
@@ -1518,10 +1518,10 @@ HandleError(JSContext* cx, InterpreterRegs& regs)
 #define POP_COPY_TO(v)           (v) = *--REGS.sp
 #define POP_RETURN_VALUE()       REGS.fp()->setReturnValue(*--REGS.sp)
 
-#define FETCH_OBJECT(cx, n, obj)                                              \
+#define FETCH_OBJECT(cx, n, obj, key)                                         \
     JS_BEGIN_MACRO                                                            \
         HandleValue val = REGS.stackHandleAt(n);                              \
-        obj = ToObjectFromStack((cx), (val));                                 \
+        obj = ToObjectFromStackForPropertyAccess((cx), (val), (key));         \
         if (!obj)                                                             \
             goto error;                                                       \
     JS_END_MACRO
@@ -2824,7 +2824,7 @@ CASE(JSOP_STRICTDELPROP)
                   "delprop and strictdelprop must be the same size");
     ReservedRooted<jsid> id(&rootId0, NameToId(script->getName(REGS.pc)));
     ReservedRooted<JSObject*> obj(&rootObject0);
-    FETCH_OBJECT(cx, -1, obj);
+    FETCH_OBJECT(cx, -1, obj, id);
 
     ObjectOpResult result;
     if (!DeleteProperty(cx, obj, id, result))
@@ -2845,9 +2845,8 @@ CASE(JSOP_STRICTDELELEM)
                   "delelem and strictdelelem must be the same size");
     /* Fetch the left part and resolve it to a non-null object. */
     ReservedRooted<JSObject*> obj(&rootObject0);
-    FETCH_OBJECT(cx, -2, obj);
-
     ReservedRooted<Value> propval(&rootValue0, REGS.sp[-1]);
+    FETCH_OBJECT(cx, -2, obj, propval);
 
     ObjectOpResult result;
     ReservedRooted<jsid> id(&rootId0);
@@ -3111,7 +3110,7 @@ CASE(JSOP_STRICTSETELEM)
                   "setelem and strictsetelem must be the same size");
     HandleValue receiver = REGS.stackHandleAt(-3);
     ReservedRooted<JSObject*> obj(&rootObject0);
-    obj = ToObjectFromStack(cx, receiver);
+    obj = ToObjectFromStackForPropertyAccess(cx, receiver, REGS.stackHandleAt(-2));
     if (!obj)
         goto error;
     ReservedRooted<jsid> id(&rootId0);
@@ -4614,7 +4613,7 @@ js::GetProperty(JSContext* cx, HandleValue v, HandlePropertyName name, MutableHa
     }
 
     RootedValue receiver(cx, v);
-    RootedObject obj(cx, ToObjectFromStack(cx, v));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, v, name));
     if (!obj)
         return false;
 
@@ -4759,7 +4758,7 @@ template <bool strict>
 bool
 js::DeletePropertyJit(JSContext* cx, HandleValue v, HandlePropertyName name, bool* bp)
 {
-    RootedObject obj(cx, ToObjectFromStack(cx, v));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, v, name));
     if (!obj)
         return false;
 
@@ -4787,7 +4786,7 @@ template <bool strict>
 bool
 js::DeleteElementJit(JSContext* cx, HandleValue val, HandleValue index, bool* bp)
 {
-    RootedObject obj(cx, ToObjectFromStack(cx, val));
+    RootedObject obj(cx, ToObjectFromStackForPropertyAccess(cx, val, index));
     if (!obj)
         return false;
 

js/src/vm/JSContext.cpp

@@ -910,11 +910,17 @@ js::ReportIsNotDefined(JSContext* cx, HandlePropertyName name)
 }
 
 void
-js::ReportIsNullOrUndefined(JSContext* cx, int spindex, HandleValue v)
+js::ReportIsNullOrUndefinedForPropertyAccess(JSContext* cx, HandleValue v, bool reportScanStack)
 {
     MOZ_ASSERT(v.isNullOrUndefined());
 
-    UniqueChars bytes = DecompileValueGenerator(cx, spindex, v, nullptr);
+    if (!reportScanStack) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CONVERT_TO,
+                                  v.isNull() ? "null" : "undefined", "object");
+        return;
+    }
+
+    UniqueChars bytes = DecompileValueGenerator(cx, JSDVG_SEARCH_STACK, v, nullptr);
     if (!bytes)
         return;
 
@@ -931,6 +937,51 @@ js::ReportIsNullOrUndefined(JSContext* cx, int spindex, HandleValue v)
     }
 }
 
+char*
+EncodeIdAsLatin1(JSContext* cx, HandleId id, JSAutoByteString& bytes)
+{
+    RootedValue idVal(cx, IdToValue(id));
+    RootedString idStr(cx, ValueToSource(cx, idVal));
+    if (!idStr)
+        return nullptr;
+
+    return bytes.encodeLatin1(cx, idStr);
+}
+
+void
+js::ReportIsNullOrUndefinedForPropertyAccess(JSContext* cx, HandleValue v, HandleId key,
+                                             bool reportScanStack)
+{
+    MOZ_ASSERT(v.isNullOrUndefined());
+
+    JSAutoByteString keyBytes;
+    if (!EncodeIdAsLatin1(cx, key, keyBytes))
+        return;
+
+    if (!reportScanStack) {
+        JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr, JSMSG_PROPERTY_FAIL,
+                                   keyBytes.ptr(),
+                                   v.isUndefined() ? js_undefined_str : js_null_str);
+        return;
+    }
+
+    UniqueChars bytes = DecompileValueGenerator(cx, JSDVG_SEARCH_STACK, v, nullptr);
+    if (!bytes)
+        return;
+
+    if (strcmp(bytes.get(), js_undefined_str) == 0 || strcmp(bytes.get(), js_null_str) == 0) {
+        JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr, JSMSG_PROPERTY_FAIL,
+                                   keyBytes.ptr(), bytes.get());
+    } else if (v.isUndefined()) {
+        JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr, JSMSG_PROPERTY_FAIL_EXPR,
+                                   bytes.get(), js_undefined_str, keyBytes.ptr());
+    } else {
+        MOZ_ASSERT(v.isNull());
+        JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr, JSMSG_PROPERTY_FAIL_EXPR,
+                                   bytes.get(), js_null_str, keyBytes.ptr());
+    }
+}
+
 void
 js::ReportMissingArg(JSContext* cx, HandleValue v, unsigned arg)
 {

js/src/vm/JSContext.h

@@ -1107,7 +1107,10 @@ ReportIsNotDefined(JSContext* cx, HandleId id);
  * Report an attempt to access the property of a null or undefined value (v).
  */
 extern void
-ReportIsNullOrUndefined(JSContext* cx, int spindex, HandleValue v);
+ReportIsNullOrUndefinedForPropertyAccess(JSContext* cx, HandleValue v, bool reportScanStack);
+extern void
+ReportIsNullOrUndefinedForPropertyAccess(JSContext* cx, HandleValue v, HandleId key,
+                                         bool reportScanStack);
 
 extern void
 ReportMissingArg(JSContext* cx, js::HandleValue v, unsigned arg);

js/src/vm/JSObject.cpp

@@ -3240,11 +3240,59 @@ js::ToObjectSlow(JSContext* cx, JS::HandleValue val, bool reportScanStack)
     MOZ_ASSERT(!val.isObject());
 
     if (val.isNullOrUndefined()) {
-        if (reportScanStack) {
+        ReportIsNullOrUndefinedForPropertyAccess(cx, val, reportScanStack);
-            ReportIsNullOrUndefined(cx, JSDVG_SEARCH_STACK, val);
+        return nullptr;
+    }
+
+    return PrimitiveToObject(cx, val);
+}
+
+JSObject*
+js::ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandleId key,
+                                  bool reportScanStack)
+{
+    MOZ_ASSERT(!val.isMagic());
+    MOZ_ASSERT(!val.isObject());
+
+    if (val.isNullOrUndefined()) {
+        ReportIsNullOrUndefinedForPropertyAccess(cx, val, key, reportScanStack);
+        return nullptr;
+    }
+
+    return PrimitiveToObject(cx, val);
+}
+
+JSObject*
+js::ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandlePropertyName key,
+                                  bool reportScanStack)
+{
+    MOZ_ASSERT(!val.isMagic());
+    MOZ_ASSERT(!val.isObject());
+
+    if (val.isNullOrUndefined()) {
+        RootedId keyId(cx, NameToId(key));
+        ReportIsNullOrUndefinedForPropertyAccess(cx, val, keyId, reportScanStack);
+        return nullptr;
+    }
+
+    return PrimitiveToObject(cx, val);
+}
+
+JSObject*
+js::ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandleValue keyValue,
+                                  bool reportScanStack)
+{
+    MOZ_ASSERT(!val.isMagic());
+    MOZ_ASSERT(!val.isObject());
+
+    if (val.isNullOrUndefined()) {
+        RootedId key(cx);
+        if (keyValue.isPrimitive()) {
+            if (!ValueToId<CanGC>(cx, keyValue, &key))
+                return nullptr;
+            ReportIsNullOrUndefinedForPropertyAccess(cx, val, key, reportScanStack);
         } else {
-            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CANT_CONVERT_TO,
+            ReportIsNullOrUndefinedForPropertyAccess(cx, val, reportScanStack);
-                                      val.isNull() ? "null" : "undefined", "object");
         }
         return nullptr;
     }

js/src/vm/JSObject.h

@@ -1206,6 +1206,38 @@ ToObjectFromStack(JSContext* cx, HandleValue vp)
     return js::ToObjectSlow(cx, vp, true);
 }
 
+JSObject*
+ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandleId key,
+                              bool reportScanStack);
+JSObject*
+ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandlePropertyName key,
+                              bool reportScanStack);
+JSObject*
+ToObjectSlowForPropertyAccess(JSContext* cx, JS::HandleValue val, HandleValue keyValue,
+                              bool reportScanStack);
+
+MOZ_ALWAYS_INLINE JSObject*
+ToObjectFromStackForPropertyAccess(JSContext* cx, HandleValue vp, HandleId key)
+{
+    if (vp.isObject())
+        return &vp.toObject();
+    return js::ToObjectSlowForPropertyAccess(cx, vp, key, true);
+}
+MOZ_ALWAYS_INLINE JSObject*
+ToObjectFromStackForPropertyAccess(JSContext* cx, HandleValue vp, HandlePropertyName key)
+{
+    if (vp.isObject())
+        return &vp.toObject();
+    return js::ToObjectSlowForPropertyAccess(cx, vp, key, true);
+}
+MOZ_ALWAYS_INLINE JSObject*
+ToObjectFromStackForPropertyAccess(JSContext* cx, HandleValue vp, HandleValue key)
+{
+    if (vp.isObject())
+        return &vp.toObject();
+    return js::ToObjectSlowForPropertyAccess(cx, vp, key, true);
+}
+
 template<XDRMode mode>
 XDRResult
 XDRObjectLiteral(XDRState<mode>* xdr, MutableHandleObject obj);

toolkit/components/extensions/test/xpcshell/test_ext_contentscript_create_iframe.js

@@ -124,7 +124,8 @@ add_task(async function test_contentscript_create_iframe() {
 
     Assert.ok(!manifest, "manifest should be undefined");
 
-    Assert.equal(String(manifestException), "TypeError: win.browser.runtime is undefined",
+    Assert.equal(String(manifestException),
+                 `TypeError: win.browser.runtime is undefined, can't access property "getManifest" of it`,
                  "expected exception received");
 
     let getManifestException = win.testGetManifestException();