[webkit-reviews] review granted: [Bug 49633] Add .responseType and .response to XMLHttpRequest : [Attachment 74603] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Chris Rogers
<crogers at google.com>'s request for review:
Bug 49633: Add .responseType and .response to XMLHttpRequest
https://bugs.webkit.org/show_bug.cgi?id=49633

Attachment 74603: Patch
https://bugs.webkit.org/attachment.cgi?id=74603&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74603&action=review

> maybe it's safer to remove it in another patch?

Sure, that's fine.

> I'm happy to remove this part of the test, but I wanted to cover the case
where I check 
> for the case when .responseType is set to "document".  Is there a simpler way
you can recommend?

I'd just check that the returned object's prototype === Document's prototype.
Or alternatively, check something like xhr.response.documentElement.tagName.

> If you think this is not a concern, then I'll change to reference a file in
the media directory.

This is a slight concern, which we usually ignore. Or you could put the file to
top level http/tests/resources directory.

Or you could just load some other file (even the .html test itself, perhaps).

> LayoutTests/fast/xmlhttprequest/xmlhttprequest-responsetype-text.html:92
> +	   if ("responseType" in xhr)
> +	       testPassed("responseType property exists.");

A very minor detail - it would be nice to have output in failure case, too. For
example, if we refer someone from Mozilla to this test, it would be nice if it
didn't hide failures.