49949 - XMLHttpRequest::onload should fire even when no XML content is returned

Status: RESOLVED WORKSFORME

(Core :: XML, defect, P3)

Description

[Taras Tielkes]

Hi Vidur, some feedback on error handling in the XmlHttpRequest component.

It looks like the current code is designed towards handling XML only. There is 
no "responseText" method to get the result text (which may or may not be XML).

http://bugzilla.mozilla.org/show_bug.cgi?id=49572

Another thing is the handling of HTTP errors, especially 404's. At the moment, 
there is no way to cache these. The IE5 solution is very simle: it's equivalent 
of "onload" will fire when the HTTP transfer is complete, with the HTTP status 
code and status text available.

To summarize, this is the chain of events I would expect:
(1) call to send()
(2) set resultText property + HTTP result code/text
(3) if result is valid xml, set responseXML property
(4) fire onload, regardless of the question if the respnse if XML

Comment 1

[Taras Tielkes]

The 6th line in the comment should of course read ".. catch these ..."

Comment 2

[Taras Tielkes]

Ack. And the last line should be "... if the response is XML.".

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Hmm... I am not sure if we should fire the "load" event. "error" might seem more
appropriate, but not sure...

Comment 4

[Taras Tielkes]

Hmm... Heikki, the IE5+ implementation will fire onload when the HTTP 
connection is "done". "done" also means HTTP errors, like a 404.

That's where the "status" and "statusText" properties come in. Checking for 
common HTTP results is trivial. I understand that you have some doubts, but 
this is the current (1.5+ year old) IE5 implementation. I think you either 
mimick it, or start your own from scratch.

My argument for following the IE5 model is that it's already out there, and 
easier for IE developers to start using. If you have a better design, this 
should be discussed on the DOM list at w3 and incorporated into a future DOM 
spec. 

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

DOM defines an error event. Mozilla already sends (I think I implemented this)
error event when an image fails to load. We do not yet send error for failed
document load (this is problematic). This is why I was considering that error
might be more approppriate. There is also abort event (which we have not yet
implemented) which should be sent when the user stops the load.

However, I do not know if 404 or some such is in fact the kind of error that
would mean we'd need to fire error event.

Still, because we are trying to mimic IE here I would not want to change things
from the way IE does them unless there are pressing reasons to do so. So I agree
with  you here.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I am taking all XMLExtras bugs to make it easier to see what I am working on...

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Targeting for mozilla0.9.1

Comment 8

[Taras Tielkes]

Heikki, you've targeted this bug for a future release.

Do you have a plan what exactly to fix, and how?

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

No plan yet, I'll get to this after mozilla0.9.

Comment 10

[Blake Ross]

Doesn't look like this is getting fixed before the freeze tomorrow night.
Pushing out a milestone.  Please correct if I'm mistaken.

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

We fire onload even for non-XML data, including 404 errors etc. There is also
responseText member now, as well as status and statusText that give you
information about the HTTP response. Marking as worksforme.