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

IE9 fix for cross domain calls #251

Merged
merged 2 commits into from Mar 6, 2016

Conversation

@ruchigoyal2005
Copy link
Contributor

commented Feb 26, 2016

This pull request includes these fixes for IE9:

  1. Fixed axios promise rejection on request onload for cors api calls.
  2. Fixed cors api calls timing out prematurely.
@nettofarah

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2016

I'm assuming that this works in all browsers (because travis is passing)
but have you manually checked this in other browsers too?

Also, can I suggest that you rewrite your commit message to make it a little more clear?
Here's a great guide on how to write good commit messages: http://chris.beams.io/posts/git-commit/

@ruchigoyal2005 ruchigoyal2005 force-pushed the ruchigoyal2005:master branch from 71cb22b to 29f5b9b Feb 29, 2016

@ruchigoyal2005 ruchigoyal2005 changed the title For IE9 this check is failing because there no response.responseText field. Request has the responseText field. so updating the condition. Fixed axios promise rejection for cors api calls in IE9 Feb 29, 2016

@ruchigoyal2005

This comment has been minimized.

Copy link
Contributor Author

commented Feb 29, 2016

I tested this change manually in Chrome/firefox/IE9/IE10/IE11. I updated the commit message. thanks for the suggestion !!!

@ruchigoyal2005 ruchigoyal2005 changed the title Fixed axios promise rejection for cors api calls in IE9 IE9 fix for cross domain calls Feb 29, 2016

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Mar 3, 2016

fixes #240

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Mar 3, 2016

How does this work with the reference to request.responseText? This seems really strange to get the response from the request.

@ruchigoyal2005

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2016

Request is XDomainRequest and it has responseText property which retrieves the response body as a string.
https://msdn.microsoft.com/en-us/library/cc288060(v=vs.85).aspx

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Mar 3, 2016

It's only XDomainRequest if it exists, otherwise it's XMLHttpRequest. Will it still work in both scenarios?

@ruchigoyal2005

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2016

@mzabriskie : For XMLHttpRequest we do get the response status. This scenario exists only for XDomainRequest. If you see this condition:

((response.status >= 200 && response.status < 300) ||
(!('status' in request) && request.responseText) ?
resolve :
reject)(response);

In 1st condition response status getting checked. For XMLHttpReuqest this 1st condition gets executed. For XDomainRequest we don't get the status in the response, that's why we need to check the request.responseText.

@ruchigoyal2005

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2016

Can you please merge this pull request if it looks ok ?..thanks !

mzabriskie added a commit that referenced this pull request Mar 6, 2016

Merge pull request #251 from ruchigoyal2005/master
IE9 fix for cross domain calls

@mzabriskie mzabriskie merged commit 84755d1 into axios:master Mar 6, 2016

1 check passed

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

This comment has been minimized.

Copy link
Member

commented Mar 6, 2016

Thank you for your PR!

@nickuraltsev

This comment has been minimized.

Copy link
Member

commented Mar 30, 2016

@ruchigoyal2005 @mzabriskie I'm still curious why this code rejects the promise in case when request.responseText is empty. For example, it's common to return an empty response body for successful PUT and DELETE requests.

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