Last Comment Bug 663749 - Remove load-event-listening code from nsXMLHttpRequest
: Remove load-event-listening code from nsXMLHttpRequest
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking)
:
Mentors:
Depends on:
Blocks: 661297
  Show dependency treegraph
 
Reported: 2011-06-12 22:51 PDT by Jonas Sicking (:sicking)
Modified: 2011-06-14 02:17 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to fix (12.03 KB, patch)
2011-06-12 22:51 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Patch to fix v2 (12.57 KB, patch)
2011-06-13 03:44 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review

Description Jonas Sicking (:sicking) 2011-06-12 22:51:23 PDT
Created attachment 538812 [details] [diff] [review]
Patch to fix

Our nsXMLHttpRequest implementation currently in some cases listens for the load event of the document being parsed. However it's unclear if this code is ever used. The comments making us wait mention that the parser could have been blocked by for example the XSLT processing code, however that code is disabled for XHR loads.

I ran this patch through try and all tests passed and the assertion didn't fire, so it seems like this patch does not affect behavior.
Comment 1 Jonas Sicking (:sicking) 2011-06-12 22:58:14 PDT
Oh, and this also removes the code that listens for error events fired on the parsed document which currently tries to make the XHR abort its load and fire an error event.

However this doesn't currently happen even if the document being loaded isn't well-formed XML. And it's also not the behavior that the XHR spec calls for.
Comment 2 Jonas Sicking (:sicking) 2011-06-13 02:52:09 PDT
Comment on attachment 538812 [details] [diff] [review]
Patch to fix

This is failing xpcshell tests. Something appears to be wrong in the error handling code.
Comment 3 Jonas Sicking (:sicking) 2011-06-13 03:44:40 PDT
Created attachment 538847 [details] [diff] [review]
Patch to fix v2

The problem is that we have xpcshell tests (and possibly internal code), which depends on the channel still being available when onerror fires. I.e. xhr.channel must not return null.
Comment 4 Jonas Sicking (:sicking) 2011-06-14 02:17:03 PDT
Checked in

http://hg.mozilla.org/mozilla-central/rev/4e1b20229d4d

There's some redness, but it's looking infrastructure related, so marking FIXED for now.

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