Last Comment Bug 726434 - Return DOMString for XMLHttpRequest.getAllResponseHeaders()
: Return DOMString for XMLHttpRequest.getAllResponseHeaders()
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-12 07:35 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2013-04-04 13:53 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.67 KB, patch)
2012-02-12 07:35 PST, :Ms2ger (⌚ UTC+1/+2)
bent.mozilla: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2012-02-12 07:35:03 PST
Created attachment 596476 [details] [diff] [review]
Patch v1

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.
Comment 1 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-02-12 10:08:16 PST
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).
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2012-02-12 10:53:00 PST
You saw

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

right?
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-02-12 11:28:22 PST
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());
  }
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-02-12 11:55:16 PST
Will do.
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2012-02-22 13:07:23 PST
https://hg.mozilla.org/mozilla-central/rev/be00940ec645

Note You need to log in before you can comment on or make changes to this bug.