Bug 15102

Summary:

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

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

New Bugs

Assignee:

Alexey Proskuryakov <ap>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, ap, roland

Priority:

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+

Sam Weinig

Reported 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.

Attachments
shortcut testcase (326 bytes, text/html) 2007-08-28 12:03 PDT, Sam Weinig	no flags	Details
addEventListener testcase (350 bytes, text/html) 2007-08-28 12:04 PDT, Sam Weinig	no flags	Details
proposed fix (5.32 KB, patch) 2007-08-29 13:15 PDT, Alexey Proskuryakov	no flags	Details Formatted Diff Diff
proposed fix (5.80 KB, patch) 2007-08-29 21:54 PDT, Alexey Proskuryakov	mjs: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Sam Weinig

Comment 1 2007-08-28 12:03:41 PDT

Created attachment 16144 [details] shortcut testcase

Sam Weinig

Comment 2 2007-08-28 12:04:16 PDT

Created attachment 16145 [details] addEventListener testcase

Alexey Proskuryakov

Comment 3 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.

Darin Adler

Comment 4 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?

Alexey Proskuryakov

Comment 5 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.

Maciej Stachowiak

Comment 6 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.

Alexey Proskuryakov

Comment 7 2007-10-02 22:08:22 PDT

Committed revision 25959 (feature branch).

Note You need to log in before you can comment on or make changes to this bug.