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.

Sign up for GitHub

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

Jump to bottom

fix(text-tracks): set withCredentials on XHR if crossOrigin='use-credentials' #6588

Merged
merged 3 commits into from
Apr 22, 2020
Merged

fix(text-tracks): set withCredentials on XHR if crossOrigin='use-credentials' #6588

merged 3 commits into from
Apr 22, 2020

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Apr 10, 2020

Fixes #6587

@gkatsev gkatsev mentioned this pull request Apr 10, 2020
Closed
@gkatsev gkatsev added the needs: LGTM Needs one or more additional approvals label Apr 17, 2020
brandonocasey
brandonocasey reviewed Apr 22, 2020
View reviewed changes
Copy link
Contributor

@brandonocasey brandonocasey left a comment

Choose a reason for hiding this comment

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

Code looks good what do you think of writing a test for this case specifically?

@gkatsev
Copy link
Member Author

gkatsev commented Apr 22, 2020

Yeah, I guess I should look into adding a test.

@gkatsev gkatsev merged commit a4ea1f9 into master Apr 22, 2020
@gkatsev gkatsev deleted the crossorigin-text-track branch April 22, 2020 20:39
@skroth
Copy link

skroth commented Apr 23, 2020

@gkatsev do you know about when we can expect to see this go out in a release?

@gkatsev
Copy link
Member Author

gkatsev commented Apr 24, 2020

Pre-release will probably be either today or next week with a full release happening about a week or more after.

@gkatsev gkatsev mentioned this pull request May 26, 2020
Open
gkatsev added a commit that referenced this pull request Jul 20, 2020
@gkatsev
fix(tech): add abstract crossOrigin method on Tech
5f823bd 
As part of #6588, we started using the crossOrigin method. However, it's
possible that a tech doesn't support this. Notably, the Flash tech. We
should instead have an abstract method on Tech that returns nothing so
we don't fail on those browsers.
@gkatsev gkatsev mentioned this pull request Jul 20, 2020
Merged
gkatsev added a commit that referenced this pull request Jul 20, 2020
@gkatsev
fix(tech): add abstract crossOrigin method on Tech (#6765)
245efac 
As part of #6588, we started using the crossOrigin method. However, it's
possible that a tech doesn't support this. Notably, the Flash tech. We
should instead have an abstract method on Tech that returns nothing so
we don't fail on those browsers.
@gkatsev gkatsev mentioned this pull request Jun 1, 2022
Open
@misteroneill misteroneill removed the needs: LGTM Needs one or more additional approvals label May 23, 2023
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
@gkatsev @edirub
fix(tech): add abstract crossOrigin method on Tech (videojs#6765)
db97421 
As part of videojs#6588, we started using the crossOrigin method. However, it's
possible that a tech doesn't support this. Notably, the Flash tech. We
should instead have an abstract method on Tech that returns nothing so
we don't fail on those browsers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandonocasey brandonocasey brandonocasey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cors to enable cookies not working for captions
4 participants
@gkatsev @skroth @brandonocasey @misteroneill