From a5758019e51581500ead1af5948868a4c79c1802 Mon Sep 17 00:00:00 2001 From: heff Date: Thu, 21 May 2015 21:46:36 -0700 Subject: [PATCH] Updated the ready event to always be async closes #1667 Fix text track tests. Now that ready is async, we need to tick the clock so the ready handler fires. Remove unneeded ready test --- CHANGELOG.md | 1 + src/js/component.js | 23 +++++++++++--------- test/globals-shim.js | 5 +++-- test/unit/component.test.js | 3 +++ test/unit/tracks/text-track-controls.test.js | 23 +++++++++++++++++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 577451fe8c..6f4a888ab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ CHANGELOG * @heff Cleaned up and documented src/js/video.js and DOM functions ([view](https://github.com/videojs/video.js/pull/2182)) * @mmcc Changed to pure CSS slider handles ([view](https://github.com/videojs/video.js/pull/2132)) * @mister-ben updated language support to handle language codes with regions ([view](https://github.com/videojs/video.js/pull/2177)) +* @heff changed the 'ready' event to always be asynchronous ([view](https://github.com/videojs/video.js/pull/2188)) -------------------- diff --git a/src/js/component.js b/src/js/component.js index 574068c1d8..f96ee75e04 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -715,7 +715,8 @@ class Component { ready(fn) { if (fn) { if (this.isReady_) { - fn.call(this); + // Ensure function is always called asynchronously + this.setTimeout(fn, 1); } else { this.readyQueue_ = this.readyQueue_ || []; this.readyQueue_.push(fn); @@ -732,20 +733,22 @@ class Component { triggerReady() { this.isReady_ = true; - let readyQueue = this.readyQueue_; + // Ensure ready is triggerd asynchronously + this.setTimeout(function(){ + let readyQueue = this.readyQueue_; - if (readyQueue && readyQueue.length > 0) { + if (readyQueue && readyQueue.length > 0) { + readyQueue.forEach(function(fn){ + fn.call(this); + }, this); - for (let i = 0; i < readyQueue.length; i++) { - readyQueue[i].call(this); + // Reset Ready Queue + this.readyQueue_ = []; } - // Reset Ready Queue - this.readyQueue_ = []; - - // Allow for using event listeners also, in case you want to do something everytime a source is ready. + // Allow for using event listeners also this.trigger('ready'); - } + }, 1); } /** diff --git a/test/globals-shim.js b/test/globals-shim.js index cbcf7b034c..df9a6d7d23 100644 --- a/test/globals-shim.js +++ b/test/globals-shim.js @@ -1,3 +1,4 @@ +import document from 'global/document'; import window from 'global/window'; import sinon from 'sinon'; @@ -6,5 +7,5 @@ window.sinon = sinon; // This may not be needed anymore, but double check before removing window.fixture = document.createElement('div'); -fixture.id = 'qunit-fixture'; -document.body.appendChild(fixture); +window.fixture.id = 'qunit-fixture'; +document.body.appendChild(window.fixture); diff --git a/test/unit/component.test.js b/test/unit/component.test.js index ed75026fe9..d851900b8f 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -360,11 +360,14 @@ test('should trigger a listener when ready', function(){ var comp = new Component(getFakePlayer(), {}, optionsReadyListener); comp.triggerReady(); + this.clock.tick(1); comp.ready(methodReadyListener); + this.clock.tick(1); // First two listeners should only be fired once and then removed comp.triggerReady(); + this.clock.tick(1); }); test('should add and remove a CSS class', function(){ diff --git a/test/unit/tracks/text-track-controls.test.js b/test/unit/tracks/text-track-controls.test.js index 8c5c312ec4..2d61a68a2c 100644 --- a/test/unit/tracks/text-track-controls.test.js +++ b/test/unit/tracks/text-track-controls.test.js @@ -2,7 +2,14 @@ import TextTrackMenuItem from '../../../src/js/control-bar/text-track-controls/t import TestHelpers from '../test-helpers.js'; import * as browser from '../../../src/js/utils/browser.js'; -q.module('Text Track Controls'); +q.module('Text Track Controls', { + 'setup': function() { + this.clock = sinon.useFakeTimers(); + }, + 'teardown': function() { + this.clock.restore(); + } +}); var track = { kind: 'captions', @@ -10,10 +17,12 @@ var track = { }; test('should be displayed when text tracks list is not empty', function() { - var player = TestHelpers.makePlayer({ + let player = TestHelpers.makePlayer({ tracks: [track] }); + this.clock.tick(1000); + ok(!player.controlBar.captionsButton.hasClass('vjs-hidden'), 'control is displayed'); equal(player.textTracks().length, 1, 'textTracks contains one item'); }); @@ -49,7 +58,11 @@ test('menu should contain "Settings", "Off" and one track', function() { var player = TestHelpers.makePlayer({ tracks: [track] }), - menuItems = player.controlBar.captionsButton.items; + menuItems; + + this.clock.tick(1000); + + menuItems = player.controlBar.captionsButton.items; equal(menuItems.length, 3, 'menu contains three items'); equal(menuItems[0].track.label, 'captions settings', 'menu contains "captions settings"'); @@ -62,6 +75,8 @@ test('menu should update with addRemoteTextTrack', function() { tracks: [track] }); + this.clock.tick(1000); + player.addRemoteTextTrack(track); equal(player.controlBar.captionsButton.items.length, 4, 'menu does contain added track'); @@ -73,6 +88,8 @@ test('menu should update with removeRemoteTextTrack', function() { tracks: [track, track] }); + this.clock.tick(1000); + player.removeRemoteTextTrack(player.textTracks()[0]); equal(player.controlBar.captionsButton.items.length, 3, 'menu does not contain removed track');