Add XHR responseType tests

[mathiasbynens]

I’ve used the same tests here: http://mathiasbynens.be/demo/xhr-responsetype

[mathiasbynens]

Ah, grunt travis was failing because in Safari/WebKit/PhantomJS setting xhr.responseType to a value it doesn’t support throws an error:

SYNTAX_ERR: DOM Exception 12: An invalid or illegal string was specified.

Gotta love unit tests. Amended the commit, wrapping the responseType assignment in a try…catch.

[patrickkettner]

Is there a reason you are using multiple tests, rather than using a bool (like the audio tests)? You'd be able to reduce the code considerably, since its basically the same test with a few different strings.

[stucox]

Personally (and @ryanseddon has said this in the past too), I prefer separate tests to using subproperties – they're easier to work with, fit better when using classes etc. But maybe this is a valid application for subproperties.

Even if sticking with multiple tests, we could reduce the duplication:

• Detects for individual request types could have an AMD dependency on Modernizr.xhrresponsetype, to avoid repeating the if (typeof XMLHttpRequest == 'undefined') { … and 'response' in xhr parts

• Could add a testXhrType() function to src/ which the individual tests could use:

  testXhrType('arraybuffer')

[mathiasbynens]

Thanks for the feedback!

@stucox I went with separate tests that don’t depend on each other because these tests are very specific. If you ever need them, chances are you only need one of them, but not more than that. Let’s say you use responseType='document' in your app and you want to detect support for it. In that situation it would be slower to run Modernizr.xhrresponsetype first, only to continue to Modernizr.xhrresponsetypedocument afterwards. Two XMLHttpRequest instances will have been created (and garbage collected) at that point, just to get the result for a single feature test. In other words, my patch is optimized for the common case where only a single responseType test is used.

TL;DR -1 to introducing an AMD dependency on Modernizr.xhrresponsetype, but +1 to adding a testXhrType() to src/. (I didn’t know that would be allowed.) I’ll tweak my pull request.

[mathiasbynens]

Amended the commit. Please review.

[stucox]

Looks good.

OCD Stu would like to see balanced gaps above and below the text in the /* DOC */ blocks though 😝 – currently you've got a space above but no space below.

[mathiasbynens]

@stucox Done. I had copied the DOC block from feature-detects/network/eventsource.js which had the same issue.

[stucox]

Top stuff. Thanks Mr Bynens!