Bug 15102

Summary: XMLHttpRequests.onreadystatechange and XMLHttpRequests.addEventListener(readystatechange, ...) should have the same behavior
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, roland
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
shortcut testcase
none
addEventListener testcase
none
proposed fix
none
proposed fix mjs: review+

Description Sam Weinig 2007-08-28 12:02:05 PDT
For XMLHttpRequests, setting an EventListener using the the shortcut notation (XHR.onreadystatechange = function()) or the addEventListener method should have the same behavior.
Comment 1 Sam Weinig 2007-08-28 12:03:41 PDT
Created attachment 16144 [details]
shortcut testcase
Comment 2 Sam Weinig 2007-08-28 12:04:16 PDT
Created attachment 16145 [details]
addEventListener testcase
Comment 3 Alexey Proskuryakov 2007-08-29 13:15:42 PDT
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.
Comment 4 Darin Adler 2007-08-29 17:12:03 PDT
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?
Comment 5 Alexey Proskuryakov 2007-08-29 21:54:02 PDT
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.
Comment 6 Maciej Stachowiak 2007-10-02 03:34:38 PDT
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.
Comment 7 Alexey Proskuryakov 2007-10-02 22:08:22 PDT
Committed revision 25959 (feature branch).