Closed Bug 726434 Opened 9 years ago Closed 9 years ago

Return DOMString for XMLHttpRequest.getAllResponseHeaders()

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(1 file)

Attached patch Patch v1Splinter Review
There is no point in doing silly things like ToNewCString(EmptyString()), and we'll probably want this for the DOM bindings work in bug 580070.
Attachment #596476 - Flags: review?(bent.mozilla)
Comment on attachment 596476 [details] [diff] [review]
Patch v1

I think this should be ok, but we need to return a void string rather than an empty string to make sure we don't break pages (they could currently test for null, who knows).
You saw

-  if (!*_retval) {
-    *_retval = ToNewCString(EmptyString());
-  }

right?
Comment on attachment 596476 [details] [diff] [review]
Patch v1

Review of attachment 596476 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, I didn't look very closely before. This seems good.

::: content/base/public/nsIXMLHttpRequest.idl
@@ +187,2 @@
>     */
> +  DOMString getAllResponseHeaders();

Needs an iid bump

::: content/base/src/nsXMLHttpRequest.cpp
@@ +1320,5 @@
>    }
> +
> +  nsRefPtr<nsHeaderVisitor> visitor = new nsHeaderVisitor();
> +  nsresult rv = httpChannel->VisitResponseHeaders(visitor);
> +  NS_ENSURE_SUCCESS(rv, NS_OK);

This is weird (and will warn when it didn't before... unclear if that's good or bad). How about:

  if (NS_SUCCEEDED(httpChannel->VisitResponseHeaders(visitor))) {
    aResponseHeaders = NS_ConvertUTF8toUTF16(visitor->Headers());
  }
Attachment #596476 - Flags: review?(bent.mozilla) → review+
Will do.
https://hg.mozilla.org/mozilla-central/rev/be00940ec645
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
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.