Bug 15102 – XMLHttpRequests.onreadystatechange and XMLHttpRequests.addEventListener(readystatechange, ...) should have the same behavior

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

For XMLHttpRequests, setting an EventListener using the the shortcut notation (XHR.onreadystatechange = function()) or the addEventListener method should have the same behavior.

[Sam Weinig]

Created attachment 16144 [details]
shortcut testcase

[Sam Weinig]

Created attachment 16145 [details]
addEventListener testcase

[Alexey Proskuryakov]

Created attachment 16158 [details]
proposed fix

I couldn't recall why I didn't implement readystatechange originally, then Sam found out that it was because Gecko 1.8 didn't support it yet, and even raised an exception when trying to add a listener for it.

[Darin Adler]

Comment on attachment 16158 [details]
proposed fix

It's be better to say evt.release() in the first call to dispatchEvent to avoid reference count churn since we don't have to use the event again after the dispatch.

If we're going to keep dispatching the same event to multiple objects, should stopPropagation on the event make us stop?

[Alexey Proskuryakov]

Created attachment 16162 [details]
proposed fix

(In reply to comment #4)
> It's be better to say evt.release() in the first call to dispatchEvent

  Done.

> If we're going to keep dispatching the same event to multiple objects, should
> stopPropagation on the event make us stop?

My understanding is that stopPropagation() only prevents traversing the DOM hierarchy, but listeners registered on the same node are invoked anyway. I've extended the test to check for this - it passes in yesterday's Firefox nightly.

[Maciej Stachowiak]

Comment on attachment 16162 [details]
proposed fix

r=me for feature-branch or when trunk reopens.

Your interpretation of the DOM is correct, stopPropagation only prevents the event from continuing to the next DOM Node.

[Alexey Proskuryakov]

Committed revision 25959 (feature branch).