274505 - 404 error does not result in xforms-link-exception for external schema

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Darin Fisher]

404 error does not result in xforms-link-exception for external schema

the problem appears to be caused by the fact that XMLHttpRequest does not
generate an "error" event for 404 responses or other HTTP error codes.  it
reserves that event for network level errors, like host not found, connection
refused, etc.

we could solve this problem several ways.  we could either fix XMLHttpRequest or
we could fix the schema loader to inspect the HTTP response code via the API
provided on XMLHttpRequest.

Comment 1

[Darin Fisher]

<darin>	do you know why XMLHttpRequest does not generate an error when a 404 is
returned?
<darin>	line 1282 of nsXMLHttpRequest.cpp only checks for NS_FAILED(status)
<darin>	seems like it should also check for nsIHttpChannel::responseSucceeded
<darin>	or is there some obscure reason why we do not do that? want to parse 404
docs perhaps?
<jst>	hmm, I can't imagine why that'd be intentional...

Comment 2

[Johnny Stenback (:jst)]

Seems to me like XMLHttpRequest needs some fixing here.

Comment 3

[Darin Fisher]

Agreed.  Patch coming up.

Comment 4

[Darin Fisher]

Actually, it seems like we should probably do what IE does.  Or, do what WhatWG
says we should do?!? :-/

Comment 5

[Darin Fisher]

Ok, it doesn't even look like MS's XMLHTTP supports the 'load' and 'error' events.

Comment 6

[Doron Rosenberg (IBM)]

As long as we keep the interfaces the same, we shouldn't have to care about IE's
implementation.

Comment 7

[Darin Fisher]

> As long as we keep the interfaces the same, we shouldn't have to care about IE's
> implementation.

interfaces imply certain behaviors that people assume when they are coding their
applications.  APIs are much more than just a set of method names and parameter
lists!

but, as i said, it might not matter since IE doesn't even seem to support the
DOM style events on the XMLHTTP object.

Comment 8

[Boris Zbarsky [:bzbarsky]]

But can you get the parsed DOM of the 404 error from IE (if the 404 returns XML)?

Comment 9

[Hixie (not reading bugmail)]

http://www.whatwg.org/specs/web-apps/current-work/#scripted says:

# If the response is an HTTP redirect, then it should be transparently followed 
# (unless it violates security or infinite loop precautions). Any other error 
# (including a 401) must cause the object to use that error page as the response.

That is, after all, what the "status" and "statusText" attributes are for.

Comment 10

[Hixie (not reading bugmail)]

It also -- not completely coincidentally -- matches what IE does:
   http://www.hixie.ch/tests/adhoc/dom/web-apps/XMLHttpRequest/014.html

Comment 11

[Hixie (not reading bugmail)]

('load' and 'error' aren't part of the WHATWG spec, since they aren't
implemented in IE. It would make sense to fire 'error' instead of 'load' for 4xx
and 5xx return codes, so long as the data is still available and in all other
respects the interface acts the way described in the spec quoted above.)

Comment 12

[Doron Rosenberg (IBM)]

xmlhttprequest.responseText will hold the 404 page's html source in Mozilla. 
Usually, testing for failure is done by checking if responseXML is null.  Then
the author knows that the loading failed.

As Hixie points out, both IE and Mozilla put the error ("404") into the status
member.

As you noticed, IE does not support onload/onerror, so you usually do a sync
load for IE or use a progresstate listener (progresstate 4 being finished).

So you are proposing that if a 404 is hit, we don't fire the onload listener? 
That could break existing content written for Mozilla, as it doesn't seem
onerror is used a lot.

Couldn't we keep existing behavior and check if the status is 200 for schema? 
Or is that unreliable?

Comment 13

[Darin Fisher]

Yeah, based on the discussion in this bug, I feel that it would be best to leave
XMLHttpRequest alone and simply fix nsSchemaLoader to check for errors properly.

Comment 14

[Doron Rosenberg (IBM)]

maybe nsXMLDocument::Load could be used instead of XMLHttpRequest?
(http://lxr.mozilla.org/seamonkey/source/content/xml/document/src/nsXMLDocument.cpp#359)

Comment 15

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

The whole idea behind XMLHttpRequest is to emulate IE's version of it as closely
as possible. We have deviated in some instances knowingly, usually after caeful
considerations of pros and cons.

The error event was coded early on, but I don't actually remember ever seeing it
happen.

HTTP errors (and success info) are available in the status, statusText and
responseText properties. I would advice checking the status value to determine
if the HTTP request succeeded.

Comment 16

[Darin Fisher]

Attachment: v1 patch

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 168726 [details] [diff] [review]
v1 patch

r+sr=jst

Comment 18

[Darin Fisher]

fixed-on-trunk (thanks everyone for the help!)