Assigning to ".onerror" of XHRs acts as though you called .addEventListener, rather than overwriting the existing event handler. This only happens inside of Web Workers. Simple test case attached; I assign the same function to "xhr.onerror" several times; the console.log output shows the handler getting called once for every time I assigned to ".onerror". I'm seeing this on Nightly (29.0a1 2013-12-11). Happy to provide more info if more clarification is needed.

For clarity, to run the test case: Save both "worker.js" and "xhr.html" somewhere, and then load the HTML file. I loaded it in a local web server (rather than from file://).

Olli?

Ah, right http://hg.mozilla.org/mozilla-central/diff/2e66e7007223/content/events/src/nsEventListenerManager.cpp#l1.32

Comment on attachment 8347424 [details] [diff] [review] worker_error_with_tests.diff Review of attachment 8347424 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/src/events/nsJSEventListener.cpp @@ +170,5 @@ > InternalScriptErrorEvent* scriptEvent = > aEvent->GetInternalNSEvent()->AsScriptErrorEvent(); > + if (scriptEvent && > + (scriptEvent->message == NS_LOAD_ERROR || > + scriptEvent->typeString.EqualsLiteral("error"))) { My main concern is that we have to test the string here now to make sure we don't have an error event, but if you're not worried about the performance then I'm not going to worry. ::: dom/workers/test/errorwarning_worker.js @@ +38,5 @@ > onerror = errorHandler; > +onerror = onerror; > +if (!onerror || onerror != onerror) { > + throw "onerror wasn't set properly"; > +} This fails before your patch, I presume?

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5) > My main concern is that we have to test the string here now to make sure we > don't have an error event, but if you're not worried about the performance > then I'm not going to worry. There is scriptEvent check already. So, no, I'm not worried about perf at all > This fails before your patch, I presume? Yes

Comment on attachment 8347424 [details] [diff] [review] worker_error_with_tests.diff [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 928312 User impact if declined: wrong behavior Testing completed (on m-c, etc.): just landed to m-i Risk to taking this patch (and alternatives if risky): Shouldn't be risky String or IDL/UUID changes made by this patch: NA