User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3 When trying to access a property of the prototype object of a DOM type, NS_ERROR_XPC_BAD_OP_ON_WN_PROTO is sometimes thrown, which cannot be caught. Sometimes DOMType.prototype.hasOwnProperty(propName) will throw an error while accessing DOMType.prototype[propName] won't, and vice-versa. Bizarrely, sometimes the error isn't thrown immediately. Accessing the property goes fine but then shortly afterwards the error is thrown. The bug can go away and reemerge from seemingly unrelated changes. Reproducible: Sometimes Steps to Reproduce: Try to access a property of the prototype object of a DOM type. examples: for( let [propName, propValue] in DOMType.prototype ) {} var obj = DOMType.prototype[propName]; DOMType.prototype.hasOwnProperty(propName); Actual Results: NS_ERROR_XPC_BAD_OP_ON_WN_PROTO is thrown. Expected Results: Nothing is thrown. The property evaluates to undefined or null if it doesn't make sense to access it from the prototype object. If the property does make sense, give the requested value. I've also tested on a development build of Firefox 3 with a clean profile, and the bug still occurred.

For me, this always throws an exception and can always be caught.

For me, this always throws an exception when refreshed and the exception can never be caught.

Can anyone confirm if this is a bug or not?

Hmm. We're reporting the error with a delay. Dunno why it's uncatchable, but not reporting it immediately is definitely a bug... There's a JS_ReportPendingException missing somewhere, clearly.

There are two parts to this fix: *) Note that the OBJ_GET_CLASS is now (correctly) on obj2 instead of obj. This was a bad typo that was causing us to throw the exception at all, obj2 doesn't have an outerObject hook. *) We will now correctly propagate outerObject failure.

So do we need this on branches too? Can we add a test added for this?

Comment on attachment 277123 [details] [diff] [review] Fix >+ JSObject *outer; >+ >+ clasp = OBJ_GET_CLASS(cx, obj2); > xclasp = (clasp->flags & JSCLASS_IS_EXTENDED) > ? (JSExtendedClass *)clasp > : NULL; >+ if (!xclasp || !xclasp->outerObject) { For some reason or other (maybe avoiding the lines, or the ternary with null else followed by null test that re-tests the ternary's condition), I prefer: clasp = OBJ_GET_CLASS(cx, obj2); if (!(clasp->flags & JSCLASS_IS_EXTENDED) || !(xclasp = (JSExtendedClass *) clasp)->outerObject) { outer = NULL; } else { r=me anyway you like. /be

Fix checked in.

Why is this nominated for the branch? Doesn't appear to be a required stability fix or regression, and no one has yet alleged any security concerns.

It's a regression from bug 316990, which we took on branch. Looks like we forgot to add the keyword when adding the dep.

blake: is this going to make 1.8.1.8? We need the fix for bug 392944 also if so, right?

I'll take blake's silence as a "no".

Blake: are you available to fix this bug for 1.8.1.12 ? Please create a branch patch (that includes regression fix bug 392944) and request branch approval. Thx.

There was a slight conflict due to s/rval/vp/ renaming, but the fix was trivial, so I'm marking this r+. This patch includes the patch for bug 392944.

Comment on attachment 294505 [details] [diff] [review] Rolled up patch approved for 1.8.1.12, a=dveditz for release-drivers

mrbkap, can you please make the one-character(!) fix from the patch in bug 392944 before you commit, so I don't have to? Thanks. /be

Already done, see comment 15, last sentence.

Fixed on the 1.8 branch.

When I look at this in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.12) Gecko/2008020121 Firefox/2.0.0.12, for the test case in comment 1, I see the same thing in 2.0.0.11 as 2.0.0.12. For the testcase in comment 2, 2.0.0.12 never shows the error in the error console while 2.0.0.11 does. Is the test case in comment 1 not good? By the behavior in the other test case (from comment 2), I would think that this is fixed but I wanted to double check.

Al, from what I can see, the testcase in comment 1 was a "control" testcase to show the exception being thrown and caught correctly. The testcase in comment 2 should no longer throw an exception thanks to the patch in this bug (the fact that we were throwing an exception at all was a bug). In conclusion, this is (as you guessed) fixed.

Thanks, Blake. I'm marking this then. I appreciate the information.

verified fixed linux|mac|windows /cvsroot/mozilla/js/tests/js1_5/extensions/regress-375344.js,v <-- regress-375344.js initial revision: 1.1