-
Notifications
You must be signed in to change notification settings - Fork 27.5k
fix($httpBackend): use ActiveX XHR when making PATCH requests on IE8 #5390
Conversation
Some quick ways to get tests passing again without getting rid of the noxhr minerr:
|
@caitp jQuery 1.x uses only the simple form: |
silly me! I didn't realize that this PR didn't pass our tests. sorry @caitp for wasting your time. the as @mzgol it should be fine to use the simplified version of the xhr instantiation. I checked with jquery source as well as some other places and this short form is legit. |
IE8's native XHR doesn't support PATCH requests, but the ActiveX one does. I'm also removing the noxhr error doc because nobody will ever get that error. Closes angular#2518 Closes angular#5043
ok, I pushed a new version now. it should be good to go. |
travis/saucelabs/karma is not having a good day today, but our ci server says that this is good to go: http://ci.angularjs.org/job/angular.js-igor/1159/ |
I don't consider it a waste of time at all --- this appears, as you said, to be a widely adopted solution, but I would still prefer to see tests where the browser can make real requests, just so that we can verify that the actual transport stuff is working as expected across all supported browsers, and have some kind of alert when if/it breaks. I don't know if there's a good infrastructure for that right now, but maybe it could be tacked on as an afterthought later, I'm not sure. But if this is good to merge, then awesome |
we don't have a good infrastructure for this kind of test right now. we could write a e2e test and add it to our build system, but it's not worth doing for this particular issue since it's an IE8 thing and you know how we feel about IE8. instead I manually tested this and verified that it works as expected and in fact I found an unrelated issue that I also fixed in fd9a03e. IE8 is such pleasure to work with... |
IE8's native XHR doesn't support PATCH requests, but the ActiveX one does.
Closes #2518
Closes #5043