Ajax: Simplify jQuery.ajaxSettings.xhr #4467
Conversation
Previously, jQuery.ajaxSettings.xhr, contents were wrapped in a try-catch as we defined jQuery.support.ajax & jQuery.support.cors executed during the jQuery load and we didn't want to crash if IE had native XHR disabled (which is possible). While jQuery hasn't supported the ActiveX-based XHR since 2.0, jQuery with XHR disabled could still be used for its other features in such a crippled browser. Since jquerygh-4347, jQuery.support.ajax & jQuery.support.cors no longer exist, so we don't need the try-catch anymore. Fixes jquerygh-1967 Ref jquerygh-4347
| try { | ||
| return new window.XMLHttpRequest(); | ||
| } catch ( e ) {} | ||
| return new window.XMLHttpRequest(); |
There was a problem hiding this comment.
In environments like jsdom the user would need to ensure there's a fake XHR stub in window or jQuery will throw an error here. That's a bit of a different behavior than currently, where it returns undefined. Any attempt to use $.ajax will fail either way, but I wonder if anyone calls $.ajaxSettings.xhr() to detect whether XHR is enabled. Probably not?
There was a problem hiding this comment.
A call to $.ajax would fail anyway. Only difference is the exception might actually be a little more informative with this change. I'm pretty certain nobody actually calls $.ajaxSettings.xhr() as a means to control if there is an XHR implementation and the change does not preclude from overriding $.ajaxSettings.xhr if needs be.
It's a +1 for me.
Summary
Previously, jQuery.ajaxSettings.xhr, contents were wrapped in a try-catch
as we defined jQuery.support.ajax & jQuery.support.cors executed during the
jQuery load and we didn't want to crash if IE had native XHR disabled (which
is possible). While jQuery hasn't supported the ActiveX-based XHR since 2.0,
jQuery with XHR disabled could still be used for its other features in such
a crippled browser.
Since gh-4347, jQuery.support.ajax & jQuery.support.cors no longer exist, so
we don't need the try-catch anymore.
Fixes gh-1967
Ref gh-4347
-8 bytes
Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com