For example, say you do Object.defineProperty(obj, "x", {...}) but it turns out obj has been frozen. The error message right now is: TypeError: x is not extensible This is wrong and stupid. Ideally, it should say TypeError: can't define obj.x: obj is not extensible We have all the tools to make this happen, we just need to hook them up. This will be more work than it looks like, but it's still a fairly quick way of making Firefox much more friendly to developers.

Hi, Jason. I'm interesting in JavaScript engine but have no experience. Is it possible for me to fix this bug?

Got some time to look at this bug, the error message is okay except my property string comes back with some ugly quotes right now. Just posting this version as a record and to ensure that I'm on the right track. The only real change I made was instructing the decompile function to search the stack. I realize this is messy and can fail in some situations (I'm curious to see if I can trick it into using the wrong variable name), but my other attempts at decompiling have turned up the actual object content. Tests are all still passing after this change. If it seems good I'll look at fixing other value error messages as well (that seems to be in scope for this bug).

Morgans-MacBook-Pro:src mrrrgn$ _DBG.OBJ/dist/bin/js js> let MmmBop = {}; js> Object.freeze(MmmBop); ({}) js> Object.defineProperty(MmmBop, "dooWop", {"value": 99}); typein:3:1 TypeError: Can't define MmmBop."dooWop": MmmBop is not extensible Stack: @typein:3:1 js>

This works, but I'm not asking for review since it makes use of JSDVG_SEARCH_STACK. I'm going to think over the edge cases and try adding 1.) tests and 2.) replacing the stack search with an actual spindex to the value decompiler. Thanks for your patience on this. >.<

This limits the change in error message to Object.defineProperty only. A call like: Object.defineProperty(frozenObj, "a", {value: 99}) will now produce an error message like: "TypeError: can't define property "a": frozenObj is not extensible" which is a considerable improvement. For other cases a slight change in the message was added to help clarify what's happening. For example, Intl.Collator.call(frozenObj) produces an error message: "Intl.Collator.call(...): object is not extensible" which I believe is slightly more accurate/clear than "Intl.Collator.call(...) is not extensible" Setting JSDVG_SEARCH_STACK helps with Object.defineProperty but breaks badly when extra arguments are added to constructor functions, that is Intl.Collator.call(frozenObj, undefined) will produce garbage like "Intl.Collator is not extensible" instead of "frozenObj is not extensible" (the message produced when there's no second argument). I looked at fixing this, but it will require changes in decompiler functions which may not be worthwhile here.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78ab553b2876

Comment on attachment 8693814 [details] [diff] [review] object-define-prop-typeerror.diff Review of attachment 8693814 [details] [diff] [review]: ----------------------------------------------------------------- r=me after much confusion. Homework: Please file a follow-up bug to fix JSDVG_IGNORE_STACK. Find a place where it's used and figure out how to trigger that error; I bet the error message will be wrong, because JSDVG_IGNORE_STACK is wrong. ::: js/src/jsapi.cpp @@ +173,5 @@ > + if (code_ == JSMSG_CANT_DEFINE_PROP_OBJECT_NOT_EXTENSIBLE) { > + RootedValue val(cx, ObjectValue(*obj)); > + return ReportValueErrorFlags(cx, flags, code_, JSDVG_SEARCH_STACK, val, > + nullptr, propName.ptr(), nullptr); > + } Let's drop this special case and do without decompilation for this particular error message. It would have been correct to allow the caller to ask for decompilation in cases where we know it's correct, like in js::obj_DefineProperty, but for now let's go for the minimum effort to get the error message reasonable...

Pushing the latest patch after tests.