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

Improve XDomainRequest implementation #185

Merged
merged 4 commits into from Jan 17, 2016

Conversation

Projects
None yet
2 participants
@jtangelder
Copy link
Contributor

jtangelder commented Jan 3, 2016

I saw the XDomainRequest detection and forsaw an issue with its implementation. The ieVersion() method would break the webworker support, since it adds a dependency to the document. My solution checks for the property withCredentials, since it was introduced at IE10.
Also removed ActiveXObject support since that is only for IE7 and older, and Axios doesn't support these, right?

And added some manual tests to verify the working in browsers. Tested with these on Saucelabs and my local machine in Win7-IE9, Win7-IE10, Win7-IE11 and Chrome 47.

jtangelder added some commits Jan 3, 2016

improve IE support
removes ActiveXObject support, and improves detection of XDomainRequest.
Add manual tests
This would help testing browser support.
@jtangelder

This comment has been minimized.

Copy link
Contributor Author

jtangelder commented Jan 3, 2016

Viewing with https://github.com/mzabriskie/axios/pull/185/files?w=1, makes line 40-60 less scary

request = null;
request.onload = function handleLoad() {
if (!request) {
return;

This comment has been minimized.

Copy link
@jtangelder

jtangelder Jan 3, 2016

Author Contributor

I don't know why this check was in the handleReadyState method. The request always contains the instance filled at this point.

@mzabriskie

This comment has been minimized.

Copy link
Member

mzabriskie commented Jan 5, 2016

@jtangelder thanks for the PR. I'm swamped this week with a project at work, but I will take a look at this as soon as I get a chance.

mzabriskie added a commit that referenced this pull request Jan 17, 2016

Merge pull request #185 from jtangelder/improve-ie
Improve XDomainRequest implementation

@mzabriskie mzabriskie merged commit be241d5 into axios:master Jan 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mzabriskie

This comment has been minimized.

Copy link
Member

mzabriskie commented Jan 17, 2016

Thanks for the PR, this makes the code a lot cleaner!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.