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

5.x fix cue-points with a startTime of 0 #4148

Merged
merged 4 commits into from Mar 2, 2017
Merged

Conversation

brandonocasey
Copy link
Contributor

Description

Wait for tech to be ready before worrying about timeupdate events

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Unit Tests updated or fixed
  • Reviewed by Two Core Contributors

@@ -331,7 +335,7 @@ class TextTrack extends Track {
addCue(originalCue) {
let cue = originalCue;

if (!(originalCue instanceof window.vttjs.VTTCue)) {
if (window.vttjs && !(originalCue instanceof window.vttjs.VTTCue)) {
Copy link
Member

Choose a reason for hiding this comment

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

I was mocking vttjs out https://github.com/videojs/video.js/pull/4146/files#diff-3b4ce30c38112a433d137fb9bf1bcbc9R21 but this probably is a better choice.

@@ -25,7 +27,7 @@ TrackBaseline(TextTrack, {
mode: 'disabled',
label: 'English',
language: 'en',
tech: defaultTech
tech: new TechFaker()
Copy link
Member

Choose a reason for hiding this comment

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

should this be this.tech?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its a suite of tests that runs on it's own, so it won't have access to this.tech as it is not in a QUnit.test

Copy link
Member

Choose a reason for hiding this comment

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

oh, I see. LGTM then.

@gkatsev gkatsev added the 5.x label Mar 2, 2017
@gkatsev gkatsev merged commit e7d4b47 into 5.x Mar 2, 2017
@gkatsev gkatsev deleted the 5.x-fix/cue-point-startTime-0 branch March 2, 2017 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants