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

Remove a bunch of code that tried to disable tracks by default #2468

Closed

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Aug 12, 2015

I think that's all of it (plus probably some tests that are forthcoming).
I'll open it against master soon.

@gkatsev
Copy link
Member Author

gkatsev commented Aug 12, 2015

Nope, looks like there aren't any tests for this that need to be removed.

@heff
Copy link
Member

heff commented Aug 12, 2015

Have we confirmed if this issue is fixed in Chrome? I got the impression it might at least be fixed in Canary. And fwiw, the captions still showed for me in desktop safari, even with the fix we're removing here. So if Chrome is fixed, not a huge impact I don't think.

lgtm

@gkatsev
Copy link
Member Author

gkatsev commented Aug 13, 2015

I'll see exactly where it is fixed, it'll be good to document. However, I still think this is the right move, for better or worse.

@gkatsev
Copy link
Member Author

gkatsev commented Aug 13, 2015

Also fixes #2436.

@heff
Copy link
Member

heff commented Aug 13, 2015

still lgtm

@forbesjo
Copy link
Contributor

👍

@gkatsev
Copy link
Member Author

gkatsev commented Aug 14, 2015

Looks like stable chrome still shows tracks by default.

gkatsev added a commit to gkatsev/video.js that referenced this pull request Aug 21, 2015
@gkatsev
Copy link
Member Author

gkatsev commented Aug 21, 2015

It's merged. a02f0be

@gkatsev gkatsev closed this Aug 21, 2015
@gkatsev gkatsev deleted the remove-track-default-cancellation branch August 21, 2015 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants