Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Bug #6298 #23

Closed
wants to merge 2 commits into from
Closed

Fix for Bug #6298 #23

wants to merge 2 commits into from

Conversation

csnover
Copy link
Member

@csnover csnover commented Sep 28, 2010

This bug has about over 9000 dupes in the bug tracker (at least 3), so here is a patch. Passes QUnit and the test case provided in duplicate #6586. I hate the JSLint assignment fix, but am not sure how to keep it from complaining about the fact that, yes, we really do want to call new XMLHttpRequest() for its side-effects.

csnover and others added 2 commits September 28, 2010 12:32
… if native XMLHttpRequest has been disabled in IE’s Group Policies. Fixes #6298.
@jeresig
Copy link
Member

jeresig commented Sep 29, 2010

Oh - I landed a fix in commit 873c284 while I was on the plane earlier - also closed #6298 at the same time. Thanks for the patch, though!

I notice that you did things a little bit differently in your implementation - I don't think we have to be quite so aggressive on making sure that we fall back to a solution (or throwing an error) - just having the one extra try/catch (as I did it) is probably fine.

@csnover
Copy link
Member Author

csnover commented Sep 29, 2010

Indeed, yes. My aims were to 1. avoid the overhead of an extra try/catch on every request, 2. keep XHR initialization all in one place, 3. improve minification slightly by using references to window.ActiveXObject and window.XMLHttpRequest, and 4. avoid any silent failures.

I agree that silent failure is not such a big deal right now since every browser that jQuery supports also supports XHR, but it might be more of an issue when thinking ahead to mobile development and the inevitable “why is ajax not working on my [whatever]” (you would be the expert on that though, I don’t know what the XHR looks like on mobiles :)).

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants