System JS : ERROR file:///Users/jruderman/Desktop/m.html:8 - ReferenceError: k is not defined

System JS : ERROR (null):0 - uncaught exception: [Opaque]

Those are using the error reporter for the XBL compilation global, not for the window. And the reason for that is that the event handler is being created in the XBL scope in marquee.xml like so: 267 var contentFn = new (XPCNativeWrapper.unwrap(window).Function)("event", aValue); 268 this["_on" + aName] = function(e) { return contentFn.call(this, e); }; ... 273 this.addEventListener(aName, this["_on" + aName], false); I guess the sane thing to do here would be to bind contentFn instead of creating a wrapper function, right? That should give us the right JSContext for error reporting. Note that I expect this also fails to properly report parse errors in aValue... And of course .onstart is totally messed up (it returns a string, not the function).

bobby, you have most recent blame on this code... I _think_ what I did there is reasonable and should not break the other stuff, but please check carefully?

Comment on attachment 8395782 [details] [diff] [review] Minimal fix. The bind() needs to be in the content compartment, sadly, not on an Xray? :( Review of attachment 8395782 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the below. ::: layout/style/xbl-marquee/xbl-marquee.xml @@ +259,5 @@ > case "string": > if (!aIgnoreNextCall) { > try { > // Work around bug 872772 by wrapping the cross-compartment- > // wrapped function in a function from this scope. So, the reason that we were wrapping in a function before wasn't to implement the bind, actually (the bind was in fact an adendum in bug 871887). It was to work around bug 872772, which is now fixed. So this comment can go away. @@ +263,5 @@ > // wrapped function in a function from this scope. > // > // NB: Make sure to wrap the constructor in parentheses to > // deal with the weird precedence of |new| in JS. > + var contentFn = new (XPCNativeWrapper.unwrap(window).Function)("event", aValue).bind(this); We should now change this to use the Xrayed Function constructor here. Note that we'll still need to waive before the call to .bind. So make this: var contentFn = XPCNativeWrapper.unwrap(new window.Function("event", aValue)).bind(this); And please file a followup bug blocked on bug 976148 to remove the waiving here.

Actually, I think this would be even better: var contentFun = new window.Function("event", aValue); var boundFunc = Function.prototype.bind.call(XPCNativeWrapper.unwrap(contentFun), this); And use boundFunc, so that we don't rely on the |bind| method in the content scope. But then we're back where we started in terms of the error in this bug. :-( (In reply to Boris Zbarsky [:bz] from comment #2) > Those are using the error reporter for the XBL compilation global, not for > the window. I think you mean the XBL scope, which is very different from the XBL compilation global.

Comment on attachment 8395782 [details] [diff] [review] Minimal fix. The bind() needs to be in the content compartment, sadly, not on an Xray? :( I think this isn't the right solution. Hang on while I set up the bugs.

The two dependent bugs here are the things we need to make this error less sucky. I've filed bug 987672 for the work in comment 6.

> But then we're back where we started in terms of the error in this bug. :-( Right, exactly. > I think you mean the XBL scope, which is very different from the XBL compilation global. I really wish we could just somehow attach the DOM error reporter to the XBL scope... So are you basically opposed to us doing anything about this bug until those deps are fixed, or do you just want the changes from comment 6?

(In reply to Boris Zbarsky [:bz] from comment #9) > So are you basically opposed to us doing anything about this bug until those > deps are fixed I think so, yeah. Error Xrays are coming soon (Q2 goal), and I'm not wild about trying to fix any of the JSErrorReporter wackiness until the underlying architecture is better. > or do you just want the changes from comment 6? The changes from comment 6 that can land now are rolled into bug 987672.

Is this still an issue? It's not clear to me what the steps to reproduce are here....

No assignee, updating the status.