Closed Bug 608939 Opened 14 years ago Closed 14 years ago

XMLHttpRequest.statusText and getAllResponseHeaders shouldn't throw

Categories

(Core :: DOM: Core & HTML, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

References

()

Details

Attachments

(2 files)

Summary: XMLHttpRequest.statusText shouldn't throw → XMLHttpRequest.statusText and getAllResponseHeaders shouldn't throw
Attached patch patchSplinter Review
Test is here http://tc.labs.opera.com/apis/XMLHttpRequest/abort-after-send.htm
I need to mochitestify it.

GetAllResponseHeaders should take nsAString, but I don't want to change
the interface at this point (hoping this minor change could actually go
to ff4).
Assignee: nobody → Olli.Pettay
Attachment #487538 - Flags: review?(jonas)
Comment on attachment 487538 [details] [diff] [review]
patch

> nsXMLHttpRequest::GetAllResponseHeaders(char **_retval)
...
>-  if (mState & XML_HTTP_REQUEST_USE_XSITE_AC) {
>-    return NS_OK;
>+  if (!(mState & XML_HTTP_REQUEST_USE_XSITE_AC)) {
>+    nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel();

Nit: Would be nice to keep the access-control if-test the way it was to match other functions. But I don't really care.

While you're here, please remove the out-of-memory check since we have infallable new. Also use an nsRefPtr<nsHeaderVisitor> instead of manually refcounting. That way you can also use NS_ENSURE_SUCCESS(rv, rv);

r=me with that.
Attachment #487538 - Flags: review?(jonas)
Attachment #487538 - Flags: review+
Attachment #487538 - Flags: approval2.0+
I don't understand the NS_ENSURE_SUCCESS(rv, rv) part.
We want to return NS_OK always.
Attached patch patch + testsSplinter Review
I need to push this to tryserver before pushing to m-c.
http://hg.mozilla.org/mozilla-central/rev/da70368cff42
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Hey, the Addon SDK test suite broke because of this change; our test ensured that XMLHttpRequest::getAllResponseHeaders() returned null when accessing a local file, but this bug appears to change its return value to an empty string. I've modified the test to accept either null or the empty string from the XHR method in bug 613071, but if you feel this was the wrong thing to do, let me know!
Based on http://dev.w3.org/2006/webapi/XMLHttpRequest/#the-getallresponseheaders-method the method returns either some valid value or empty string.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: