Made vjs.get work with xDomain in IE < 10 #1095

skogby · 2014-03-19T09:39:50Z

This fix makes the videojs get function work in older versions of IE by using XDomainRequest

mmcc · 2014-03-19T17:49:31Z

Thanks! What is the error you're seeing in the older versions of IE that this fixes?

skogby · 2014-03-20T09:50:36Z

It enables IE to send cross domain xml requests like an ad request to Google DFP:
http://pubads.g.doubleclick.net/gampad/ads?env=vp&gdfp_req=1&impl=s&output=xml_vast2&iu=/6062/video-demo&sz=400x300&unviewed_position_start=1&ciu_szs=728x90,300x250&url=[ref_url]&correlator=[timestamp]

Most modern browsers can use the "withCredentials" flag, which older IE versions does not support.

mmcc · 2014-03-20T17:17:47Z

Cool! When @heff gets back we'll take a look and see about getting this one pulled in.

heff · 2014-03-26T22:04:36Z

Very cool, thanks! I'm not familiar enough with this to confirm so going to try to find one other person to give the thumbs up.
@dmlap, @gkatsev, @BCjwhisenant - You guys have been working with cross domain stuff. Can you take a look at this?

jrw95 · 2014-03-26T23:13:13Z

We can find someone on the team to take a look tomorrow.
Thanks!

gkatsev · 2014-03-26T23:51:33Z

looks like it's the way to go http://www.html5rocks.com/en/tutorials/cors/#toc-making-a-cors-request.
I'm sure that setting onprogress to a noop function is necessary. Also, unncessary, is the need for duplicating the request.open call. It's exactly the same past the creation to the XHR/XDomainRequest object.

dmlap · 2014-03-27T00:10:00Z

src/js/lib.js

  try {
-    request.open('GET', url);
+    request.withCredentials = true;


Should this always be set?

…ture/IE-get-xDomain

skogby · 2014-03-28T07:10:27Z

The withCredentials flag allows the request to include cookies, but could be configured with a parameter. Just noted yesterday as well that IE11 requires the request to be open before we can set the withCredentials flag.
request.open('GET', url);
request.withCredentials = true;

gkatsev · 2014-03-28T17:41:44Z

That's weird that IE11 requires that. However, yes, it should be a configuration. We don't want to always set credentials to true because sometimes cookies are disabled or whatever.

heff · 2014-04-14T18:00:22Z

@skogby do we still need to update this to open the request before setting withCredentials? And can we make it so that setting is configurable?

gkatsev · 2014-04-14T18:05:09Z

Yes, we only want the withCredentials flag set if required. Otherwise, a lot of requests will fail.

skogby · 2014-04-15T07:10:35Z

Just pushed some updates that changed the order of "open" and "withCredentials" in addition to making it not being sent as default.

floehopper · 2014-05-02T14:24:29Z

I think I'm seeing this problem too with IE9 and subtitles hosted on a different sub-domain. Are these changes likely to get merged and released any time soon?

gkatsev · 2014-05-02T19:04:17Z

Looks good. Can you also add the withCredentials flag (if needed) to the XDomainRequest? Line 590?

floehopper · 2014-05-04T13:17:04Z

@gkatsev XDomainRequest does not support withCredentials, so I don't think any change is needed. @skogby is that right?

floehopper · 2014-05-04T13:20:10Z

As I mentioned above, I've run into this problem on IE9 when I have subtitles hosted on a different sub-domain. I've just built a test page using the code in this PR and I can confirm that it fixes my problem. Is there anything I can do to help get this merged and released?

gkatsev · 2014-05-04T17:10:05Z

Ah, cool. For some reason I thought it did support that property. Carry on then.

The commit integrates the changes in [1] which are in turn based on this video.js pull request [2]. This should fix a problem whereby cross-domain Ajax GET requests from within video.js did not work for Internet Explorer < v10. This should mean that subtitles (which are hosted on a sub-domain of `futurelearn.com` in production, i.e. `ugc.futurelearn.com`) work with the video.js player on earlier versions of Internet Explorer. [1] https://github.com/Futurelearn/video.js/compare/v4.5.2-with-IE-get-xDomain-fix [2] videojs/video.js#1095

heff · 2014-05-06T23:35:05Z

This is working for me in everything but IE8, where it errors out now when loading captions. Can anyone else test this in IE8 and verify?

If you run a local server and test the index.html file (copied from index.html.example) in /sandbox/, you should be able to play the video, then turn on captions and see captions. But this now breaks in IE8.

I logged out the onError function in vjs.get, and in true IE8 fashion, no information is passed to the error handler. So I have no idea what might be causing this. But it is clear that IE8 is now using XDomainRequest in instead of the XMLHttpRequest/Msxml2.XMLHTTP shim it previously was. If there was a way to continue that when IE8 is used, that might fix this issue. I'm not sure if that's the best approach though.

heff · 2014-05-06T23:56:33Z

So to clarify a little, IE8 does support native XMLHttpRequests (xhr1 not xhr2). So apparently XDomainRequest doesn't work in all the same situations XMLHttpRequest does. Are there clear times where we should be using one and not the other?

gkatsev · 2014-05-07T01:49:34Z

Basically, XDomainRequest is needed because XHR 1 doesn't understand CORS headers. So, XDomainRequest should be used whenever you're making a request to another domain, which is set up with CORS headers.

However, I think XDRs should be able to do whatever XHR 1 does for same domain requests as well (as long as the requests are simple as only GET and POST are supported by XDR.

heff · 2014-05-07T03:42:18Z

That's good to know. But for some reason XDR isn't working in the index.html example, while XHR is. If someone can try the example and verify that'd be helpful, so I know it's not just me.

Found this writeup, but I don't believe any of these should be affecting the index.html example.

XDomainRequest Gotchas
Some other things I came across that will save you a headache.
The server protocol must be the same as the calling page protocol.
This means you can’t make requests from file:// to http ://, http to https, or https to http (don’t ever do the latter)
I found this the hard way by trying to run my code locally without first spinning up my localhost server.
Only “text/plain” is supported for the request’s “Content Type header
No custom headers can be added to the request

Added a more explicit MIME type note in setup. Moved to a footnote and made comment more verbose to avoid confusion.

floehopper · 2014-05-08T10:57:33Z

@heff I haven't had a chance to try it on the example, but I have my own version working OK in IE8 including subtitles. In fact the reason I needed the fix was because my subtitles are hosted on a different domain. I don't know whether it's significant, but I'm instantiating the player via JS and passing a tracks array in the options. I'll try to test the example later today if get a chance.

heff · 2014-05-08T20:49:57Z

Thanks @floehopper, that'd be helpful. I tried forcing my local server to send text/plain for vtt files, and that didn't help the issue. And IE8 is still passing zero information to the onerrror handler, so I'm not sure what to try next. I don't think anythings out of the ordinary with my example, especially since xmlhttprequest is working. I wonder if it's worth checking if the file is cross domain and only using xdomainrequest in that case.

…js into dbmedialab-feature/IE-get-xDomain

Did some reorganization of the code.

heff · 2014-05-08T23:01:29Z

I made some updates and submitted a pull request to this branch. dbmedialab-archive#1

If anyone can test if these changes still work for them, that'd be great.

floehopper · 2014-05-09T15:00:20Z

@heff As I've mentioned [1] over on your PR, subtitles don't work any more on a different sub-domain since your most recent changes.

[1] dbmedialab-archive#1 (comment)

Changed to used XDomainRequest only when url is cross domain Tested and it seems to be working, I currently don't have access to a IE9 computer though.

heff · 2014-05-12T23:40:56Z

Ok, I think I have this fixed in dbmedialab-archive#2

floehopper · 2014-05-13T10:28:37Z

I'm going to test this now.

floehopper · 2014-05-13T10:53:02Z

Just to re-iterate what I've said on @heff's latest PR, I've created a new test page [1] for my use case (where subtitles are on a sub-domain) and tested that subtitles work OK on the following: Win XP/IE8, Win7/IE9, Win8/IE10, Win8.1/IE11, OSX10.9/Firefox29, OSX10.9/Safari7 & OSX10.9/Chrome34. Good work and thanks!

[1] http://code.gofreerange.com/video-test/video.js-vzaar-subtitles-3.html

heff · 2014-05-13T17:31:34Z

Woo! Thanks for testing it out. Going to pull this in then.

heff · 2014-05-13T19:39:23Z

This has been merged into master and will go out with the next minor release.

Made vjs.get work in IE < 10

86e2dfd

mmcc added the enhancement label Mar 20, 2014

dmlap reviewed Mar 27, 2014
View reviewed changes

src/js/lib.js

try {

request.open('GET', url);

request.withCredentials = true;

Copy link

Member

dmlap Mar 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this always be set?

Merge branch 'master' of https://github.com/videojs/video.js into fea…

8691077

…ture/IE-get-xDomain

Erik Skogby added 2 commits April 15, 2014 09:00

Fixed a bug with withcredentials

440de37

Set the withCredentials flag as an optional parameter

b7311f3

heff and others added 3 commits May 7, 2014 15:23

Added line to changelog

3fd0c92

Merge branch 'master' of github.com:videojs/video.js

bcb69a0

More explicit MIME type note

92eac13

Added a more explicit MIME type note in setup. Moved to a footnote and made comment more verbose to avoid confusion.

heff added 2 commits May 8, 2014 15:22

Merge branch 'feature/IE-get-xDomain' of github.com:dbmedialab/video.…

24459eb

…js into dbmedialab-feature/IE-get-xDomain

Switched to only using XDomainRequest if it is cross domain.

0812660

Did some reorganization of the code.

Merge pull request #1 from heff/dbmedialab-feature/IE-get-xDomain

5d76663

Changed to used XDomainRequest only when url is cross domain Tested and it seems to be working, I currently don't have access to a IE9 computer though.

heff closed this in 047bd8f May 13, 2014

heff mentioned this pull request May 19, 2014

Add needed info to text track docs #837

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Made vjs.get work with xDomain in IE < 10 #1095

Made vjs.get work with xDomain in IE < 10 #1095

skogby commented Mar 19, 2014

mmcc commented Mar 19, 2014

skogby commented Mar 20, 2014

mmcc commented Mar 20, 2014

heff commented Mar 26, 2014

jrw95 commented Mar 26, 2014

gkatsev commented Mar 26, 2014

dmlap Mar 27, 2014

skogby commented Mar 28, 2014

gkatsev commented Mar 28, 2014

heff commented Apr 14, 2014

gkatsev commented Apr 14, 2014

skogby commented Apr 15, 2014

floehopper commented May 2, 2014

gkatsev commented May 2, 2014

floehopper commented May 4, 2014

floehopper commented May 4, 2014

gkatsev commented May 4, 2014

heff commented May 6, 2014

heff commented May 6, 2014

gkatsev commented May 7, 2014

heff commented May 7, 2014

floehopper commented May 8, 2014

heff commented May 8, 2014

heff commented May 8, 2014

floehopper commented May 9, 2014

heff commented May 12, 2014

floehopper commented May 13, 2014

floehopper commented May 13, 2014

heff commented May 13, 2014

heff commented May 13, 2014

Made vjs.get work with xDomain in IE < 10 #1095

Made vjs.get work with xDomain in IE < 10 #1095

Conversation

skogby commented Mar 19, 2014

mmcc commented Mar 19, 2014

skogby commented Mar 20, 2014

mmcc commented Mar 20, 2014

heff commented Mar 26, 2014

jrw95 commented Mar 26, 2014

gkatsev commented Mar 26, 2014

dmlap Mar 27, 2014

Choose a reason for hiding this comment

skogby commented Mar 28, 2014

gkatsev commented Mar 28, 2014

heff commented Apr 14, 2014

gkatsev commented Apr 14, 2014

skogby commented Apr 15, 2014

floehopper commented May 2, 2014

gkatsev commented May 2, 2014

floehopper commented May 4, 2014

floehopper commented May 4, 2014

gkatsev commented May 4, 2014

heff commented May 6, 2014

heff commented May 6, 2014

gkatsev commented May 7, 2014

heff commented May 7, 2014

floehopper commented May 8, 2014

heff commented May 8, 2014

heff commented May 8, 2014

floehopper commented May 9, 2014

heff commented May 12, 2014

floehopper commented May 13, 2014

floehopper commented May 13, 2014

heff commented May 13, 2014

heff commented May 13, 2014