[Webkit-unassigned] [Bug 54162] XHR inconsistent in handling access to response attributes after an error

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54162





--- Comment #5 from Jarred Nicholls <jarred.nicholls at gmail.com>  2011-02-12 22:28:09 PST ---
(From update of attachment 81933)
View in context: https://bugs.webkit.org/attachment.cgi?id=81933&action=review

Overall, the idea here is to make XHR (this is XHR1 spec, not XHR2) to-spec.  It "effectively" behaves the same as IE & Opera, not giving any response data on errors or at incorrect times.  The difference is that it always returns a value and doesn't throw DOM exceptions.

While what you say (about split code paths for IE & Firefox) has some truth in general, I assure you it has zero truth for XHR.  I work on one of the largest JS libraries in the world, and I have used all of the other major libraries out there.  When it comes to handling XHR, there isn't a split code path, ever.  No one is saying "well if Firefox, then yeah let's get this response data even though I just threw an error, else don't do it".  It just doesn't happen.  No one tries to get this data after an error, particularly because IE has never allowed it.

The point is, if IE disallows access to this data, and the spec says that the data shouldn't be accessed, then *no one is trying to do it*.

Keep in mind that the only "effective net difference" this patch has to WebKit is access to status & statusText after an error would no longer give a value.  WebKit throws an exception on getAllResponseHeaders() and getResponseHeader() (therein lies the inconsistency).

Hope that helps explain the position.  Thanks!

> LayoutTests/http/tests/xmlhttprequest/exceptions.html:-68
> -    shouldThrow('req.statusText()');

status and statusText will never throw an exception, so they are not tested for an expected exception.

> LayoutTests/http/tests/xmlhttprequest/getAllResponseHeaders.html:39
> +        }

getAllResponseHeaders() will always return a string.  It will either be a string of the concatenated headers (state >= HEADERS_RECEIVED && !error), or an empty string, but a string nonetheless.  So we test for a string on every call to getAllResponseHeaders().  Essentially any other behavior that is encountered is a failed test case.

IE & Opera: throws exceptions after error
Firefox: allows access after error

> LayoutTests/http/tests/xmlhttprequest/getResponseHeader-expected.txt:19
> +PASSED 1 Content-Type: Content-Type is null.

getResponseHeader() will either return the header string (state >= HEADERS_RECEIVED && !error && headerExists) or null.  It no longer throws exceptions when accessed at the incorrect time.

IE & Opera: throws exception after error
Firefox: allows access after error

> LayoutTests/http/tests/xmlhttprequest/status-after-abort-expected.txt:7
> +Sent request. Response status: 0; statusText: ''; readyState: 1

status and statusText no longer throw exceptions when accessed at the wrong time.  They will either return the status code and status text (state >= HEADERS_RECEIVED && !error), or they will return 0 and "", respectively.  This includes the "abort" error.  An error is an error...

IE & Opera: throws exceptions after error
Firefox: allows access after error

> LayoutTests/http/tests/xmlhttprequest/zero-length-response-expected.txt:21
> +- ResponseXML serialized: n/a

Likewise, see above comments on status and statusText.  This test is specifically testing responsetext, but needed to be updated for status/statusText since they don't throw exceptions.

> LayoutTests/http/tests/xmlhttprequest/zero-length-response.html:91
> +       try { log ("- ResponseXML serialized: " + (req.responseXML ? prettyPrintText(window.XMLSerializer ? (new XMLSerializer()).serializeToString(req.responseXML) : req.responseXML.xml) : "n/a")); } catch (ex) { log("Â Â Exception serializing ResponseXML: " + ex.message); }

Someone used stupid spaces, so I made it more legible ascii :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.