From 60b5db7da7c6e69aa6637bda89f7f5bb33df4df5 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 23 Nov 2016 14:26:57 -0500 Subject: [PATCH 01/15] test(player): use assert.throws for custom player --- test/unit/player.test.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index e2e8ef7d46..62f00f4efa 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1276,12 +1276,9 @@ QUnit.test('should not allow to register custom player when any player has been const player = videojs(tag); class CustomPlayer extends Player {} - try { + assert.throws(function() { videojs.registerComponent('Player', CustomPlayer); - } catch (e) { - player.dispose(); - return assert.equal(e.message, 'Can not register Player component after player has been created'); - } + }, 'Can not register Player component after player has been created'); - assert.ok(false, 'It should throw Error when any player has been created'); + player.dispose(); }); From 0e2984576aefac2669f521ca5725df8a538a58dc Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 23 Nov 2016 14:48:08 -0500 Subject: [PATCH 02/15] test --- src/js/component.js | 6 ++++++ test/unit/player.test.js | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/js/component.js b/src/js/component.js index 36b3d6e031..6c12ba23b8 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1440,9 +1440,15 @@ class Component { Component.components_ = {}; } + if (window.foo) { + console.log(name, Component.components_[name]); + } if (name === 'Player' && Component.components_[name]) { const Player = Component.components_[name]; + if (window.foo) { + console.log(Object.keys(Player.players)); + } if (Player.players && Object.keys(Player.players).length > 0) { throw new Error('Can not register Player component after player has been created'); } diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 62f00f4efa..89b1da576f 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1272,6 +1272,7 @@ QUnit.test('should allow to register custom player when any player has not been }); QUnit.test('should not allow to register custom player when any player has been created', function(assert) { + window.foo = true; const tag = TestHelpers.makeTag(); const player = videojs(tag); @@ -1281,4 +1282,5 @@ QUnit.test('should not allow to register custom player when any player has been }, 'Can not register Player component after player has been created'); player.dispose(); + delete window.foo; }); From 668f6e8716bda21656e8ab4a6795a1f2f13a42f3 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 23 Nov 2016 14:49:33 -0500 Subject: [PATCH 03/15] window --- src/js/component.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 6c12ba23b8..0f13c8df08 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1441,13 +1441,13 @@ class Component { } if (window.foo) { - console.log(name, Component.components_[name]); + window.console.log(name, Component.components_[name]); } if (name === 'Player' && Component.components_[name]) { const Player = Component.components_[name]; if (window.foo) { - console.log(Object.keys(Player.players)); + window.console.log(Object.keys(Player.players)); } if (Player.players && Object.keys(Player.players).length > 0) { throw new Error('Can not register Player component after player has been created'); From 41d9fc23db449bc2c253f745eab7959db4f2ae88 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 23 Nov 2016 14:59:56 -0500 Subject: [PATCH 04/15] remove console/window. Switch to function custom player --- src/js/component.js | 6 ------ test/unit/player.test.js | 5 ++--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 0f13c8df08..36b3d6e031 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1440,15 +1440,9 @@ class Component { Component.components_ = {}; } - if (window.foo) { - window.console.log(name, Component.components_[name]); - } if (name === 'Player' && Component.components_[name]) { const Player = Component.components_[name]; - if (window.foo) { - window.console.log(Object.keys(Player.players)); - } if (Player.players && Object.keys(Player.players).length > 0) { throw new Error('Can not register Player component after player has been created'); } diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 89b1da576f..8ad85eab26 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1272,15 +1272,14 @@ QUnit.test('should allow to register custom player when any player has not been }); QUnit.test('should not allow to register custom player when any player has been created', function(assert) { - window.foo = true; const tag = TestHelpers.makeTag(); const player = videojs(tag); - class CustomPlayer extends Player {} + const CustomPlayer = function() {}; + assert.throws(function() { videojs.registerComponent('Player', CustomPlayer); }, 'Can not register Player component after player has been created'); player.dispose(); - delete window.foo; }); From 155c0a4c5ebeb75358058744bd2fc7a502051512 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 14:50:20 -0500 Subject: [PATCH 05/15] reset the Player component after a test --- test/unit/player.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 8ad85eab26..b8fa13d3f5 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1269,6 +1269,9 @@ QUnit.test('should allow to register custom player when any player has not been assert.equal(player instanceof CustomPlayer, true, 'player is custom'); player.dispose(); + + // reset the Player to the original value; + videojs.registerComponent('Player', Player); }); QUnit.test('should not allow to register custom player when any player has been created', function(assert) { @@ -1282,4 +1285,7 @@ QUnit.test('should not allow to register custom player when any player has been }, 'Can not register Player component after player has been created'); player.dispose(); + + // reset the Player to the original value; + videojs.registerComponent('Player', Player); }); From e828c612de2c8d50a612a02c90774edaf8948b5d Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 14:51:00 -0500 Subject: [PATCH 06/15] swap CustomPlayer back to class --- test/unit/player.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index b8fa13d3f5..f3801f5727 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1278,7 +1278,7 @@ QUnit.test('should not allow to register custom player when any player has been const tag = TestHelpers.makeTag(); const player = videojs(tag); - const CustomPlayer = function() {}; + class CustomPlayer extends Player {} assert.throws(function() { videojs.registerComponent('Player', CustomPlayer); From e53c140a35f3df39d6f6cd363cf5b73b5a3a791a Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 15:22:12 -0500 Subject: [PATCH 07/15] If length is greater than 1, it could still be null --- src/js/component.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/js/component.js b/src/js/component.js index 36b3d6e031..5f67c147d7 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1443,7 +1443,9 @@ class Component { if (name === 'Player' && Component.components_[name]) { const Player = Component.components_[name]; - if (Player.players && Object.keys(Player.players).length > 0) { + if (Player.players && + Object.keys(Player.players).length > 0 && + Object.keys(Player.players).map((name) => Player.players[name]).every(Boolean)) { throw new Error('Can not register Player component after player has been created'); } } From d257f424d71497b2af9d5c06f7189ed4854014b0 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 15:23:02 -0500 Subject: [PATCH 08/15] name->playerName --- src/js/component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/component.js b/src/js/component.js index 5f67c147d7..cb8c7862c2 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1445,7 +1445,7 @@ class Component { if (Player.players && Object.keys(Player.players).length > 0 && - Object.keys(Player.players).map((name) => Player.players[name]).every(Boolean)) { + Object.keys(Player.players).map((playerName) => Player.players[playerName]).every(Boolean)) { throw new Error('Can not register Player component after player has been created'); } } From 2df1ee4ccdcb855db7733699c23fa9a8e26c0e25 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 15:55:06 -0500 Subject: [PATCH 09/15] use .innerText on IE8 --- test/unit/tracks/text-track-controls.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/tracks/text-track-controls.test.js b/test/unit/tracks/text-track-controls.test.js index 526694ae9e..c5d6b45954 100644 --- a/test/unit/tracks/text-track-controls.test.js +++ b/test/unit/tracks/text-track-controls.test.js @@ -408,7 +408,8 @@ test('chapters menu should use track label as menu title', function() { player.controlBar.chaptersButton.update(); const menu = player.controlBar.chaptersButton.menu; - const menuTitle = menu.contentEl().firstChild.textContent; + const titleEl = menu.contentEl().firstChild; + const menuTitle = titleEl.textContent || titleEl.innerText; equal(menuTitle, 'Test Chapters', 'menu gets track label as title'); From eb07f69e73dcfecf1690741f9fd77b056b375018 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 15:55:24 -0500 Subject: [PATCH 10/15] remove windows line endings --- test/unit/test-helpers.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/unit/test-helpers.js b/test/unit/test-helpers.js index 65429c7e8e..8a8f8b4311 100644 --- a/test/unit/test-helpers.js +++ b/test/unit/test-helpers.js @@ -139,22 +139,22 @@ const TestHelpers = { * @param {string} eventType */ triggerDomEvent(element, eventType) { - let event; - - if (document.createEvent) { - event = document.createEvent('HTMLEvents'); - event.initEvent(eventType, true, true); - } else { - event = document.createEventObject(); - event.eventType = eventType; - } - - event.eventName = eventType; - - if (document.createEvent) { - element.dispatchEvent(event); - } else { - element.fireEvent('on' + event.eventType, event); + let event; + + if (document.createEvent) { + event = document.createEvent('HTMLEvents'); + event.initEvent(eventType, true, true); + } else { + event = document.createEventObject(); + event.eventType = eventType; + } + + event.eventName = eventType; + + if (document.createEvent) { + element.dispatchEvent(event); + } else { + element.fireEvent('on' + event.eventType, event); } } }; From 8ff5f2827802979797fc720c4fa8844af75c4972 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 16:37:50 -0500 Subject: [PATCH 11/15] trigger load directly if emulated tracks --- test/unit/tracks/text-track-controls.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/tracks/text-track-controls.test.js b/test/unit/tracks/text-track-controls.test.js index c5d6b45954..0fcc88b2a5 100644 --- a/test/unit/tracks/text-track-controls.test.js +++ b/test/unit/tracks/text-track-controls.test.js @@ -436,7 +436,11 @@ test('chapters should be displayed when remote track added and load event fired' equal(chaptersEl.track.cues.length, 2); - TestHelpers.triggerDomEvent(chaptersEl, 'load'); + if (player.tech_.featuresNativeTextTracks) { + TestHelpers.triggerDomEvent(chaptersEl, 'load'); + } else { + chaptersEl.trigger('load'); + } ok(!player.controlBar.chaptersButton.hasClass('vjs-hidden'), 'chapters menu is displayed'); From fd567897f7aef44e3f71ef8f518ab82c040f70eb Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 17:59:51 -0500 Subject: [PATCH 12/15] make sure we dispose of players --- test/unit/player.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/unit/player.test.js b/test/unit/player.test.js index f3801f5727..8d325a67c2 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -144,6 +144,8 @@ QUnit.test('should get current source from source tag', function(assert) { assert.ok(player.currentSource().src === 'http://google.com'); assert.ok(player.currentSource().type === 'video/mp4'); + + player.dispose(); }); QUnit.test('should get current sources from source tag', function(assert) { @@ -172,6 +174,8 @@ QUnit.test('should get current sources from source tag', function(assert) { assert.ok(player.currentSources()[0].src === 'http://google.com'); assert.ok(player.currentSources()[0].type === undefined); assert.ok(player.currentSources()[1] === undefined); + + player.dispose(); }); QUnit.test('should get current source from src set', function(assert) { @@ -208,6 +212,7 @@ QUnit.test('should get current source from src set', function(assert) { assert.ok(player.currentSource().src === 'http://google.com'); assert.ok(player.currentSource().type === 'video/mp4'); + player.dispose(); }); QUnit.test('should get current sources from src set', function(assert) { @@ -255,6 +260,8 @@ QUnit.test('should get current sources from src set', function(assert) { assert.ok(player.currentSources()[0].src === 'http://hugo.com'); assert.ok(player.currentSources()[0].type === undefined); assert.ok(player.currentSources()[1] === undefined); + + player.dispose(); }); QUnit.test('should asynchronously fire error events during source selection', function(assert) { @@ -334,6 +341,8 @@ QUnit.test('should default to 16:9 when fluid', function(assert) { // IE8 rounds 0.5625 up to 0.563 assert.ok(((ratio >= 0.562) && (ratio <= 0.563)), 'fluid player without dimensions defaults to 16:9'); + + player.dispose(); }); QUnit.test('should set fluid to true if element has vjs-fluid class', function(assert) { @@ -344,6 +353,8 @@ QUnit.test('should set fluid to true if element has vjs-fluid class', function(a const player = TestHelpers.makePlayer({}, tag); assert.ok(player.fluid(), 'fluid is true with vjs-fluid class'); + + player.dispose(); }); QUnit.test('should use an class name that begins with an alpha character', function(assert) { From d6d95e16b33a0dd74020da6358a95b034873f198 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 28 Nov 2016 18:35:10 -0500 Subject: [PATCH 13/15] Bump From 75faaca53f94ed453d6f250ba780ccb1e3fb53b6 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 29 Nov 2016 11:34:47 -0500 Subject: [PATCH 14/15] add comment complaining Player section in registerComponent --- src/js/component.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/js/component.js b/src/js/component.js index cb8c7862c2..09fcba9f94 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -1443,6 +1443,10 @@ class Component { if (name === 'Player' && Component.components_[name]) { const Player = Component.components_[name]; + // If we have players that were disposed, then their name will still be + // in Players.players. So, we must loop through and verify that the value + // for each item is not null. This allows registration of the Player component + // after all players have been disposed or before any were created. if (Player.players && Object.keys(Player.players).length > 0 && Object.keys(Player.players).map((playerName) => Player.players[playerName]).every(Boolean)) { From 03c147d8774a95c5b19836894ede16daf778aea5 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 29 Nov 2016 12:02:11 -0500 Subject: [PATCH 15/15] add chapters el comment --- test/unit/tracks/text-track-controls.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit/tracks/text-track-controls.test.js b/test/unit/tracks/text-track-controls.test.js index 0fcc88b2a5..05122fecab 100644 --- a/test/unit/tracks/text-track-controls.test.js +++ b/test/unit/tracks/text-track-controls.test.js @@ -436,6 +436,10 @@ test('chapters should be displayed when remote track added and load event fired' equal(chaptersEl.track.cues.length, 2); + // Anywhere where we support using native text tracks, we can trigger a custom DOM event. + // On IE8 and other places where we have emulated tracks, either we cannot trigger custom + // DOM events (like IE8 with the custom DOM element) or we aren't using a DOM element at all. + // In those cases just trigger `load` directly on the chaptersEl object. if (player.tech_.featuresNativeTextTracks) { TestHelpers.triggerDomEvent(chaptersEl, 'load'); } else {