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

Doesn't auto choose XDR when making a cross-domain request #94

Open
gkatsev opened this issue Sep 23, 2015 · 8 comments
Open

Doesn't auto choose XDR when making a cross-domain request #94

gkatsev opened this issue Sep 23, 2015 · 8 comments
Labels

Comments

@gkatsev
Copy link

gkatsev commented Sep 23, 2015

As a user of XHR, I would expect it to realize when the request will require CORS and choose to use XDR. This is mostly an issue on browsers like IE8 and IE9 where a CORS request requires the usage of XDomainRequest.

@naugtur
Copy link
Owner

naugtur commented Sep 23, 2015

Thanks for reporting. I had that issue too. Over a year ago, when I was not involved with this project yet.

It's a wontfix kind of issue.
It only (not mostly) affects really old IEs and if you are using xdr you have to be aware of a lot of limitations anyway. Eg. Can't set or read any headers.

Detecting if the request is cross domain is also not as easy as we'd want to. So it'd be a big chunk of code added for eberyone while only very few users have any doubts if they are crossing domain boundaries or not on a particular request.

Having said all that, I'm open to pull requests with a small and elegant solution I couldn't come up with ;)
But it'd have to be complete. No half measures like handling only absolute urls or so. It should require no documentation.

@gkatsev
Copy link
Author

gkatsev commented Sep 23, 2015

Do you think something like https://github.com/videojs/video.js/blob/stable/src/js/xhr.js#L80-L84 is sufficient or are there some more usecase you can think off that won't be covered?

@gkatsev
Copy link
Author

gkatsev commented Sep 23, 2015

There's a better implementation than what I linked above in videojs/video.js#2633

@naugtur
Copy link
Owner

naugtur commented Sep 23, 2015

That's still a lot of code for a library of the size of xhr anyway. We could distribute a separate library with that feature (assuming it doesn't exist already)

it's like with the qs package. We got requests to include that too, but it's better off as an optional thing you can add.

@gkatsev
Copy link
Author

gkatsev commented Sep 23, 2015

What about making it in an require('xhr/ie')? or require('xhr/cors')?

@naugtur
Copy link
Owner

naugtur commented Sep 24, 2015

Yes, xhr/ie was considered before. This is yet another reason to finally do that.
Dropping all that ie-induced mess to a separate file would be such a relief.

@naugtur naugtur added the wontfix label Jul 8, 2016
@ArtskydJ
Copy link
Contributor

Should this be closed?

@naugtur
Copy link
Owner

naugtur commented Nov 18, 2016

I'm keeping it open because people don't search closed issues and this is a wontfix.
I'll close it when we separate out old IE support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants