707484 - [XHR2] Allow setting XHR responseType and withCredentials before open

Status: RESOLVED FIXED

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

Description

[Masatoshi Kimura [:emk]]

The latest XHR2 spec allows setting responseType and withCredentials in UNSENT or OPENED state.

Comment 1

[Tillmann Karras]

Attachment: Allow setting XHR.responseType before open()

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 707601 [details] [diff] [review]
Allow setting XHR.responseType before open()

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

r=me if you also add a test.

Comment 3

[Tillmann Karras]

Attachment: Allow setting XHR.responseType before open()

HRs were not asynchronous by default. I also added a test.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 708091 [details] [diff] [review]
Allow setting XHR.responseType before open()

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

r=me with that fixed.

::: content/base/test/test_XHR.html
@@ +90,5 @@
>    try { xhr.responseXML } catch (e) { didthrow = true; }
>    ok(didthrow, "should have thrown when accessing responseXML");
>  }
> +function checkSetResponseTypeThrows(xhr, type, shouldThrow) {
> +  var shouldThrow = shouldThrow === undefined ? true : shouldThrow;

shouldThrow = !!shouldThrow;

@@ +101,3 @@
>  }
> +function checkOpenThrows(xhr, method, url, async, shouldThrow) {
> +  var shouldThrow = shouldThrow === undefined ? true : shouldThrow;

Same

@@ +118,5 @@
> +checkSetResponseTypeThrows(xhr, "moz-chunked-text", false);
> +checkSetResponseTypeThrows(xhr, "moz-chunked-arraybuffer", false);
> +checkOpenThrows(xhr, "GET", "file_XHR_pass2.txt", false, true);
> +checkSetResponseTypeThrows(xhr, "", false);
> +checkOpenThrows(xhr, "GET", "file_XHR_pass2.txt", false, false);

Also add a test that checks that setting the reponseType to something before calling .open() actually works. I.e. that it interprets the response according to the set responseType.

Comment 5

[Masatoshi Kimura [:emk]]

Is it safe to change .responseType in HEADER_RECEIVED state?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

yup, that should already work fine.

Comment 7

[Hallvord R. M. Steen [:hallvors]]

Setting responseType throws an unexpected error in these test cases:
http://w3c-test.org/web-platform-tests/master/XMLHttpRequest/open-method-responsetype-set-sync.htm
http://w3c-test.org/web-platform-tests/master/XMLHttpRequest/response-json.htm
http://w3c-test.org/web-platform-tests/master/XMLHttpRequest/responsexml-non-document-types.htm

I'm going to assume this bug is the reason.

Comment 8

[Hallvord R. M. Steen [:hallvors]]

Here's a withCredentials test case:
http://w3c-test.org/web-platform-tests/master/XMLHttpRequest/withcredentials-set.htm

Comment 9

[Masatoshi Kimura [:emk]]

Tillmann, could you post a new patch with review comments resolved?

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://tbpl.mozilla.org/?tree=Try&rev=3bfc5e786606

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://tbpl.mozilla.org/?tree=Try&rev=fde331808597

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Allow setting XHR.responseType before open()

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This is Tillmann's patch rebased to trunk. I removed the 

> checkSetResponseTypeThrows(xhr, "", false);
> checkOpenThrows(xhr, "GET", "file_XHR_pass2.txt", false, false);

bit from the test; it's already covered by the wpt tests. I left the rest in because it also tests proprietary values that the wpt tests can't.

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5bec86469c8

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b5bec86469c8

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Tillmann, thanks for the patch!