XMLHttpRequest.responseXML should return null when a network error occurs

[jdm]

Spec: https://xhr.spec.whatwg.org/#document-response step 1
Code: components/script/dom/xmlhttprequest.rs
Test:

• ./mach test-wpt tests/wpt/web-platform-tests/xhr/xmlhttprequest-network-error.htm
• ./mach test-wpt tests/wpt/web-platform-tests/xhr/xmlhttprequest-network-error-sync.htm

We fail the assert_equals(client.responseXML, null, "responseXML") check, because we don't check if self.response_status is an Err value in the document_response method before creating a document response to return.

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[petosorus]

Hi!
I'm pretty new to rust and would like to check I'm getting it.

1. Should the check of response_status be done between the response.is_some() block and the mime_type block?
2. If the response_status matches Err, should the return be something like

self.response_xml.set(null);
return self.response_xml.get();

Thanks for putting up these tasks marked as Easy :)

[jdm]

1. Yes
2. There should not be any need to set self.response_xml, since it will be empty already. Returning None directly will be most readable.

[CYBAI]

Hi @petosorus , thanks for looking into this!

1. Should the check of response_status be done between the `response.is_some()` block and the `mime_type` block?

I think it might be worth mentioning that, with reading the commit of if response.is_some() { (347f3c1), it's for caching.

So, as @jdm answered, yes, the check of response_status should be done between response.is_some() and mime_type block.

Maybe it's worth updating the comment of response.is_some() checking and say Step 1 above response_status checking instead? (@jdm what do you think?)

Ex.

- // Step 1
+ // Caching: if we have existing response xml, redirect it directly
let response = self.response_xml.get();
...

+ // Step 1.
<!-- the response_status check -->

[jdm]

Yes, that would be an improvement!

[petosorus]

Ok, that's cool with me! I'll start tonight then.

@highfive: assign me

[highfive]

Hey @petosorus! Thanks for your interest in working on this issue. It's now assigned to you!