Look like a ua sniff, is that a really best way to do this?

It's a browser sniff, not UA sniff (changes in UA won't make this code behave differently). Also, this will not catch any new browsers but only old, specific ones (i.e. only IE8 or newer IE in IE8 document mode) so this will not have any unwanted effect on new browsers.

But if you have any idea how to test for this specific issue, I'm listening. :) I just don't think it's possible to test for such weird cases in JS code, especially without seriously impacting performance.

Yea, I think we'd all prefer feature tests, but we've resigned ourselves to do this in a few necessary places. This one seems legit.

This all got quite convoluted. How about reversing the polarity?

// Native XHR cannot access local files, always use ActiveX for that case
return this.isLocal ||

    // Also force ActiveX for non-RFC2616 methods with IE<9 (trac-13240)
    // http://msdn.microsoft.com/en-us/library/ie/ms536648(v=vs.85).aspx
    // ...but *ONLY* IE<9, since IE 10-11 cross-domain PATCH requires native XHR (gh-1684)
    document.documentMode < 9 && !/^(?:get|post|head|put|delete|options)$/i.test( this.type ) ?

    createActiveXHR() :
    createStandardXHR() || createActiveXHR();

That seems more readable but what's the point of the last || createActiveXHR() in your example?

Since we're talking about this, how about simple if or else here and there, ternaries might not be a good choice for conditions like this.

It shouldn't hit bytes count too much or not at all

Why don't we specifically tests for "patch"?

( document.documentMode < 9 && /^patch$/i.test( this.type ) || createStandardXHR() ) && createActiveXHR();

( document.documentMode < 9 && /^patch$/i.test( this.type ) || createStandardXHR() ) && createActiveXHR();

IE<9 doesn't support any non-RFC2616 method as indicated in the comment. I don't think testing for PATCH is enough.

I'm okay with the document.documentMode check. Depending on when 3.0 ships I am starting to think we might just want to patch the 1.x line and not even have a compat branch, since we will drop it soon thereafter.

Uhoh, can of worms opened.

But for now, I think it's best to stick with the solution we have.

Only let's have an couple if here not ternary with || and && abomination

Sure. I'll modify the patch.

So now we need a internet connection to run the tests and rely on the third party?

This test requires a real cross-domain PATCH request. So it's either internet connection required or a separate server set up locally (which most people won't do, it's an additional hassle) that responds to PATCH requests and sends CORS headers.

Apart from using a 3rd party service we may define a separate domain with a new nginx config just for this test. @gnarf doesn't have time to work on it now and even if I sent him an nginx config for this new server, this would still be the only externally visible server with CORS enabled that's responding to cross-domain requests. This is a security risk so it would have to be done carefully; that's why @gnarf suggested to use an external service.

Do you have any other ideas?

Best way would be to run http-server, but until we have php-dep with swarm we can't do it.

I'd say stubbing is another option to consider, i remember we talked about introducing these kind of testing for special cases, it will also resolve your browserstack issue.

Another way would be to test jQuery.ajaxSettings.xhr, not the whole ajax call.

How would you test jQuery.ajaxSettings.xhr? And what stubbing do you propose? You cannot test if a native API bugs is worked around properly if you replace those native APIs with stubs that won't reproduce these bugs.

The same exact thing you could do with stubbing interface without introducing concerns above.

So what do you propose? To add a couple of stubs that represent IE8 (no cross-domain support, PATCH only via ActiveX) and IE10-11 (cross-domain PATCH only via native XHR) and test them? IMO it's incredibly hard to create such stubs and be sure everything is replicated properly.

We could try, i'm fine with landing it as is, afterwards, i could try to do this myself if you found this to hard.

Would like others to agree with my view though.

i could try to do this myself if you found this to hard.

It's not hard to implement (just a little tedious), it's hard to be sure the whole behavior is replicated properly. Especially that IE11 up until recently was receiving more substantial patches so sth like that could have changed behind us and we wouldn't detect it if we just tested on stubs. Maybe now that's not an issue (@dmethvin probably know more) but I still feel uneasy about not testing this on real browsers instead of just our assumptions about how they work.

As I understand it, Spartan will have just about all the IE-isms removed. So it should be safe to use .documentMode because Spartan won't even have document modes.

Also, a detection via if ( window.ActiveXObject ) { ... } fails in IE11 because it was used for bad browser sniffs. You could use it to your advantage in this case perhaps? Actually I guess not because IE10 should use the native XHR.

Also, a detection via if ( window.ActiveXObject ) { ... } fails in IE11 because it was used for bad browser sniffs.

window.ActiveXObject !== undefined will succeed, though since we use it ourselves. ;) But this would be stepping over our implementation, document.documentMode seems safer here.

Not sure about this change. We do have special logic for all IE versions now but PATCH is broken again on BrowserStack in some IEs. Removing it completely sounds dangerous, though until we have integration tests in place.

Update: this wasn't working for me as I have the extension installed and then in the Live interface PATCH is not supported. It should work fine on our TestSwarm testing