From 332e89632d38ca4f9a16d400ee07a32b5272026d Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 28 Mar 2017 23:20:25 -0400 Subject: [PATCH 01/32] resizer --- src/css/video-js.scss | 10 ++++++++++ src/js/player.js | 4 +++- src/js/resize-manager.js | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 src/js/resize-manager.js diff --git a/src/css/video-js.scss b/src/css/video-js.scss index 0ae831c22e..1df2ea9077 100644 --- a/src/css/video-js.scss +++ b/src/css/video-js.scss @@ -42,3 +42,13 @@ @import "components/captions-settings"; @import "print"; + +.vjs-resize-manager { + position: absolute; + top: 0; + left: 0; + width: 100%; + height: 100%; + border: none; + visibility: hidden; +} diff --git a/src/js/player.js b/src/js/player.js index f6d2552cd2..4b8b1e264d 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -44,6 +44,7 @@ import './close-button.js'; import './control-bar/control-bar.js'; import './error-display.js'; import './tracks/text-track-settings.js'; +import './resize-manager.js'; // Import Html5 tech, at least for disposing the original video tag. import './tech/html5.js'; @@ -3471,7 +3472,8 @@ Player.prototype.options_ = { 'bigPlayButton', 'controlBar', 'errorDisplay', - 'textTrackSettings' + 'textTrackSettings', + 'resizeManager' ], language: navigator && (navigator.languages && navigator.languages[0] || navigator.userLanguage || navigator.language) || 'en', diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js new file mode 100644 index 0000000000..aef443f2b4 --- /dev/null +++ b/src/js/resize-manager.js @@ -0,0 +1,33 @@ +/** + * @file resize-manager.js + */ +import Component from './component.js'; + +class ResizeManager extends Component { + constructor(player, options) { + super(player, options); + + this.el_.addEventListener('load', () => { + this.el_.contentWindow.addEventListener('resize', (event) => this.resizeHandler(event)); + }); + } + + createEl() { + return super.createEl('iframe', { + className: 'vjs-resize-manager' + }); + } + + resizeHandler(event) { + // this.player_.trigger('playerresize'); + + const width = this.player_.currentWidth(); + + console.log(width, width/10, width/50, width/100); + + // this.player_.el_.style.fontSize = width/50 + 'px'; + } +} + +Component.registerComponent('ResizeManager', ResizeManager); +export default ResizeManager; From 92adf24de6d69fb7eb5272fe1bdb90c36827f893 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Sun, 7 Jan 2018 23:09:01 -0500 Subject: [PATCH 02/32] wip --- src/js/resize-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index aef443f2b4..5ff6aa7ecf 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -25,7 +25,7 @@ class ResizeManager extends Component { console.log(width, width/10, width/50, width/100); - // this.player_.el_.style.fontSize = width/50 + 'px'; + this.player_.el_.style.fontSize = width/50 + 'px'; } } From 2ea72f91f5d84555208675b140cd3880d8be951b Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Mon, 8 Jan 2018 17:25:08 -0500 Subject: [PATCH 03/32] use resize observer or iframe --- src/js/resize-manager.js | 44 ++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 5ff6aa7ecf..73383b29cd 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -1,32 +1,60 @@ /** * @file resize-manager.js */ +import window from 'global/window'; +import * as Fn from './utils/fn.js'; import Component from './component.js'; +const RESIZE_OBSERVER_AVAILABLE = window.ResizeObserver; + class ResizeManager extends Component { constructor(player, options) { super(player, options); - this.el_.addEventListener('load', () => { - this.el_.contentWindow.addEventListener('resize', (event) => this.resizeHandler(event)); - }); + if (RESIZE_OBSERVER_AVAILABLE) { + this.resizeObserver = new window.ResizeObserver(() => this.resizeHandler()); + this.resizeObserver.observe(player.el()); + + } else { + this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); + + const loadListener = () => { + this.el_.contentWindow.addEventListener('resize', this.iframeResizeHandler_); + this.el_.removeEventListener('load', loadListener); + }; + + this.el_.addEventListener('load', loadListener); + } } createEl() { + if (RESIZE_OBSERVER_AVAILABLE) { + return; + } + return super.createEl('iframe', { className: 'vjs-resize-manager' }); } - resizeHandler(event) { - // this.player_.trigger('playerresize'); + resizeHandler() { + this.player_.trigger('playerresize'); + } - const width = this.player_.currentWidth(); + dispose() { + if (this.resizeObserver) { + this.resizeObserver.unobserve(this.player.el()); + this.resizeObserver.disconnect(); + } - console.log(width, width/10, width/50, width/100); + if (this.iframeResizeHandler_) { + this.el_.contentWindow.removeEventListener('resize', this.iframeResizeHandler_); + } - this.player_.el_.style.fontSize = width/50 + 'px'; + this.resizeObserver = null; + this.iframeResizeHandler_ = null; } + } Component.registerComponent('ResizeManager', ResizeManager); From e7fa09b112f8c73267c080f162e25fdef9130390 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 13:57:13 -0500 Subject: [PATCH 04/32] Revert "feat: playerresize event on Player dimension API calls (#4800)" This reverts commit e0ed0b5cd762f83dc54a7b56e8774718485e924d. --- src/js/player.js | 32 +++++--------------------------- test/unit/player.test.js | 29 ----------------------------- 2 files changed, 5 insertions(+), 56 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 4b8b1e264d..f16ef51f9e 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -662,14 +662,11 @@ class Player extends Component { * @param {number} [value] * The value to set the `Player`'s width to. * - * @param {boolean} [skipListeners] - * Skip the playerresize event trigger - * * @return {number} * The current width of the `Player` when getting. */ - width(value, skipListeners) { - return this.dimension('width', value, skipListeners); + width(value) { + return this.dimension('width', value); } /** @@ -679,21 +676,16 @@ class Player extends Component { * @param {number} [value] * The value to set the `Player`'s heigth to. * - * @param {boolean} [skipListeners] - * Skip the playerresize event trigger - * * @return {number} * The current height of the `Player` when getting. */ - height(value, skipListeners) { - return this.dimension('height', value, skipListeners); + height(value) { + return this.dimension('height', value); } /** * A getter/setter for the `Player`'s width & height. * - * @fires Player#playerresize - * * @param {string} dimension * This string can be: * - 'width' @@ -702,13 +694,10 @@ class Player extends Component { * @param {number} [value] * Value for dimension specified in the first argument. * - * @param {boolean} [skipListeners] - * Skip the playerresize event trigger - * * @return {number} * The dimension arguments value when getting (width/height). */ - dimension(dimension, value, skipListeners) { + dimension(dimension, value) { const privDimension = dimension + '_'; if (value === undefined) { @@ -731,17 +720,6 @@ class Player extends Component { this[privDimension] = parsedVal; this.updateStyleEl_(); - - // skipListeners allows us to avoid triggering the resize event when setting both width and height - if (this.isReady_ && !skipListeners) { - /** - * Triggered when the player is resized. - * - * @event Player#playerresize - * @type {EventTarget~Event} - */ - this.trigger('playerresize'); - } } /** diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 31bd9d82b9..0c357497a6 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1709,32 +1709,3 @@ QUnit.test('player.duration() sets the value of player.cache_.duration', functio player.duration(200); assert.equal(player.duration(), 200, 'duration() set and get integer duration value'); }); - -QUnit.test('should fire playerresize when player is resized', function(assert) { - assert.expect(2); - - const player = TestHelpers.makePlayer(); - - player.on('playerresize', function() { - assert.ok(true, 'playerresize fired'); - }); - - player.width(400); - player.height(300); - - player.dispose(); -}); - -QUnit.test('should fire playerresize exactly once for a two-dimensional resize', function(assert) { - assert.expect(1); - - const player = TestHelpers.makePlayer(); - - player.on('playerresize', function() { - assert.ok(true, 'playerresize fired once'); - }); - - player.dimensions(400, 300); - - player.dispose(); -}); From 5902483213c94511b8779b88ebd70af79b3fde5d Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 14:13:25 -0500 Subject: [PATCH 05/32] make sure that contentWindow is available before remove listener --- src/js/resize-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 73383b29cd..535e0efdc8 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -47,7 +47,7 @@ class ResizeManager extends Component { this.resizeObserver.disconnect(); } - if (this.iframeResizeHandler_) { + if (this.iframeResizeHandler_ && this.el_.contentWindow) { this.el_.contentWindow.removeEventListener('resize', this.iframeResizeHandler_); } From 439ac9e7f0af8ffb3815123495ea24038fb6e25a Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 15:03:50 -0500 Subject: [PATCH 06/32] dispose iframe load handler properly --- src/js/resize-manager.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 535e0efdc8..d28363cd0a 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -11,19 +11,22 @@ class ResizeManager extends Component { constructor(player, options) { super(player, options); + this.iframeResizeHandler_ = null; + this.loadListener_ = null; + this.resizeObserver = null; + if (RESIZE_OBSERVER_AVAILABLE) { this.resizeObserver = new window.ResizeObserver(() => this.resizeHandler()); this.resizeObserver.observe(player.el()); } else { this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); - - const loadListener = () => { + this.loadListener_ = () => { this.el_.contentWindow.addEventListener('resize', this.iframeResizeHandler_); - this.el_.removeEventListener('load', loadListener); + this.el_.removeEventListener('load', this.loadListener_); }; - this.el_.addEventListener('load', loadListener); + this.el_.addEventListener('load', this.loadListener_); } } @@ -51,8 +54,13 @@ class ResizeManager extends Component { this.el_.contentWindow.removeEventListener('resize', this.iframeResizeHandler_); } + if (this.loadListener_) { + this.el_.removeEventListener('load', this.loadListener_); + } + this.resizeObserver = null; this.iframeResizeHandler_ = null; + this.loadListener_ = null; } } From c98a945e335169299e0aa0717b79dc15c6622148 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 15:38:43 -0500 Subject: [PATCH 07/32] dispose some players in tests when done --- test/unit/controls.test.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index 0a2a94f288..b0e7a9b55c 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -31,6 +31,7 @@ QUnit.test('should hide volume control if it\'s not supported', function(assert) assert.ok(volumeControl.hasClass('vjs-hidden'), 'volumeControl is not hidden'); assert.ok(muteToggle.hasClass('vjs-hidden'), 'muteToggle is not hidden'); + player.dispose(); }); @@ -56,6 +57,8 @@ QUnit.test('should test and toggle volume control on `loadstart`', function(asse assert.equal(volumeControl.hasClass('vjs-hidden'), false, 'volumeControl does not show itself'); assert.equal(muteToggle.hasClass('vjs-hidden'), false, 'muteToggle does not show itself'); + + player.dispose(); }); QUnit.test('calculateDistance should use changedTouches, if available', function(assert) { @@ -78,6 +81,8 @@ QUnit.test('calculateDistance should use changedTouches, if available', function }; assert.equal(slider.calculateDistance(event), 0.5, 'we should have touched exactly in the center, so, the ratio should be half'); + + player.dispose(); }); QUnit.test('should hide playback rate control if it\'s not supported', function(assert) { @@ -87,6 +92,7 @@ QUnit.test('should hide playback rate control if it\'s not supported', function( const playbackRate = new PlaybackRateMenuButton(player); assert.ok(playbackRate.el().className.indexOf('vjs-hidden') >= 0, 'playbackRate is not hidden'); + player.dispose(); }); @@ -97,9 +103,11 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t player.isFullscreen(true); player.trigger('fullscreenchange'); assert.equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); + player.isFullscreen(false); player.trigger('fullscreenchange'); assert.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); + player.dispose(); }); @@ -118,6 +126,8 @@ if (Html5.isSupported()) { assert.equal(player.volume(), 1, 'volume is same'); assert.equal(player.muted(), true, 'player is muted'); + + player.dispose(); }); QUnit.test('Clicking MuteToggle when volume is 0 and muted is false should set volume to lastVolume and keep muted false', function(assert) { @@ -132,6 +142,8 @@ if (Html5.isSupported()) { assert.equal(player.volume(), 1, 'volume is set to lastVolume'); assert.equal(player.muted(), false, 'muted remains false'); + + player.dispose(); }); QUnit.test('Clicking MuteToggle when volume is 0 and muted is true should set volume to lastVolume and sets muted to false', function(assert) { @@ -146,6 +158,8 @@ if (Html5.isSupported()) { assert.equal(player.volume(), 0.5, 'volume is set to lastVolume'); assert.equal(player.muted(), false, 'muted is set to false'); + + player.dispose(); }); QUnit.test('Clicking MuteToggle when volume is 0, lastVolume is less than 0.1, and muted is true sets volume to 0.1 and muted to false', function(assert) { @@ -161,6 +175,8 @@ if (Html5.isSupported()) { // `Number.prototype.toFixed()` is used here to circumvent IE9 rounding issues assert.equal(player.volume().toFixed(1), (0.1).toFixed(1), 'since lastVolume is less than 0.1, volume is set to 0.1'); assert.equal(player.muted(), false, 'muted is set to false'); + + player.dispose(); }); QUnit.test('ARIA value of VolumeBar should start at 100', function(assert) { @@ -170,6 +186,8 @@ if (Html5.isSupported()) { this.clock.tick(1); assert.equal(volumeBar.el_.getAttribute('aria-valuenow'), 100, 'ARIA value of VolumeBar is 100'); + + player.dispose(); }); QUnit.test('Muting with MuteToggle should set ARIA value of VolumeBar to 0', function(assert) { @@ -193,5 +211,7 @@ if (Html5.isSupported()) { assert.equal(player.volume(), 1, 'Volume remains 1'); assert.equal(player.muted(), true, 'Muted is true'); assert.equal(volumeBar.el_.getAttribute('aria-valuenow'), 0, 'ARIA value of VolumeBar is 0'); + + player.dispose(); }); } From 5219730c52bc2bf7510f4ef1f6487b507d6dc6af Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 16:09:29 -0500 Subject: [PATCH 08/32] dispose all the things --- test/api/api.js | 4 ++ test/unit/button.test.js | 3 +- test/unit/component.test.js | 55 +++++++++++++++++++++++++++ test/unit/menu.test.js | 2 + test/unit/poster.test.js | 8 ++++ test/unit/tracks/audio-tracks.test.js | 1 + test/unit/tracks/text-tracks.test.js | 4 ++ 7 files changed, 76 insertions(+), 1 deletion(-) diff --git a/test/api/api.js b/test/api/api.js index 934a4f63e3..aa4a71d5a5 100644 --- a/test/api/api.js +++ b/test/api/api.js @@ -118,6 +118,8 @@ QUnit.test('should be able to access expected component API methods', function(a assert.ok(comp.clearInterval, 'clearInterval exists'); assert.ok(comp.setTimeout, 'setTimeout exists'); assert.ok(comp.clearTimeout, 'clearTimeout exists'); + + comp.dispose(); }); QUnit.test('should be able to access expected MediaTech API methods', function(assert) { @@ -289,4 +291,6 @@ QUnit.test('should extend Component', function(assert) { const noMethods = new NoMethods({}); assert.ok(noMethods.on, 'should extend component with no methods or constructor'); + + myComponent.dispose(); }); diff --git a/test/unit/button.test.js b/test/unit/button.test.js index 9902cb4253..8a1429f1cc 100644 --- a/test/unit/button.test.js +++ b/test/unit/button.test.js @@ -24,6 +24,7 @@ QUnit.test('should localize its text', function(assert) { assert.ok(el.nodeName.toLowerCase().match('button')); assert.ok(el.innerHTML.match(/vjs-control-text"?>Juego/)); assert.equal(el.getAttribute('title'), 'Juego'); - player.dispose(); + testButton.dispose(); + player.dispose(); }); diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 00e59f5b32..db35e8d887 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -86,6 +86,8 @@ QUnit.test('should create an element', function(assert) { const comp = new Component(getFakePlayer(), {}); assert.ok(comp.el().nodeName); + + comp.dispose(); }); QUnit.test('should add a child component', function(assert) { @@ -98,6 +100,8 @@ QUnit.test('should add a child component', function(assert) { assert.ok(comp.el().childNodes[0] === child.el()); assert.ok(comp.getChild('component') === child); assert.ok(comp.getChildById(child.id()) === child); + + comp.dispose(); }); QUnit.test('should add a child component to an index', function(assert) { @@ -129,6 +133,8 @@ QUnit.test('should add a child component to an index', function(assert) { assert.ok(comp.children().length === 5); assert.ok(comp.children()[3] === child3); assert.ok(comp.children()[4] === child2); + + comp.dispose(); }); QUnit.test('addChild should throw if the child does not exist', function(assert) { @@ -138,6 +144,7 @@ QUnit.test('addChild should throw if the child does not exist', function(assert) comp.addChild('non-existent-child'); }, new Error('Component Non-existent-child does not exist'), 'addChild threw'); + comp.dispose(); }); QUnit.test('addChild with instance should allow getting child correctly', function(assert) { @@ -151,6 +158,8 @@ QUnit.test('addChild with instance should allow getting child correctly', functi comp.addChild(comp2); assert.ok(comp.getChild('foo'), 'we can get child with camelCase'); assert.ok(comp.getChild('Foo'), 'we can get child with TitleCase'); + + comp.dispose(); }); QUnit.test('should add a child component with title case name', function(assert) { @@ -163,6 +172,8 @@ QUnit.test('should add a child component with title case name', function(assert) assert.ok(comp.el().childNodes[0] === child.el()); assert.ok(comp.getChild('Component') === child); assert.ok(comp.getChildById(child.id()) === child); + + comp.dispose(); }); QUnit.test('should init child components from options', function(assert) { @@ -174,6 +185,8 @@ QUnit.test('should init child components from options', function(assert) { assert.ok(comp.children().length === 1); assert.ok(comp.el().childNodes.length === 1); + + comp.dispose(); }); QUnit.test('should init child components from simple children array', function(assert) { @@ -187,6 +200,8 @@ QUnit.test('should init child components from simple children array', function(a assert.ok(comp.children().length === 3); assert.ok(comp.el().childNodes.length === 3); + + comp.dispose(); }); QUnit.test('should init child components from children array of objects', function(assert) { @@ -200,6 +215,8 @@ QUnit.test('should init child components from children array of objects', functi assert.ok(comp.children().length === 3); assert.ok(comp.el().childNodes.length === 3); + + comp.dispose(); }); QUnit.test('should do a deep merge of child options', function(assert) { @@ -236,6 +253,7 @@ QUnit.test('should do a deep merge of child options', function(assert) { // Reset default component options to none Component.prototype.options_ = null; + comp.dispose(); }); QUnit.test('should init child components from component options', function(assert) { @@ -246,6 +264,8 @@ QUnit.test('should init child components from component options', function(asser assert.ok(!testComp.childNameIndex_.TestComponent2, 'we do not have testComponent2'); assert.ok(testComp.childNameIndex_.TestComponent4, 'we have a testComponent4'); + + testComp.dispose(); }); QUnit.test('should allows setting child options at the parent options level', function(assert) { @@ -271,6 +291,7 @@ QUnit.test('should allows setting child options at the parent options level', fu } assert.equal(parent.children()[0].options_.foo, true, 'child options set when children array is used'); assert.equal(parent.children().length, 1, 'we should only have one child'); + parent.dispose(); // using children object options = { @@ -294,6 +315,7 @@ QUnit.test('should allows setting child options at the parent options level', fu } assert.equal(parent.children()[0].options_.foo, true, 'child options set when children object is used'); assert.equal(parent.children().length, 1, 'we should only have one child'); + parent.dispose(); }); QUnit.test('should dispose of component and children', function(assert) { @@ -349,6 +371,8 @@ QUnit.test('should add and remove event listeners to element', function(assert) comp.trigger('test-event'); comp.off('test-event', testListener); comp.trigger('test-event'); + + comp.dispose(); }); QUnit.test('should trigger a listener once using one()', function(assert) { @@ -363,6 +387,8 @@ QUnit.test('should trigger a listener once using one()', function(assert) { comp.one('test-event', testListener); comp.trigger('test-event'); comp.trigger('test-event'); + + comp.dispose(); }); QUnit.test('should be possible to pass data when you trigger an event', function(assert) { @@ -381,6 +407,8 @@ QUnit.test('should be possible to pass data when you trigger an event', function comp.one('test-event', testListener); comp.trigger('test-event', {d1: data1, d2: data2}); comp.trigger('test-event'); + + comp.dispose(); }); QUnit.test('should add listeners to other components and remove them', function(assert) { @@ -452,6 +480,9 @@ QUnit.test('should add listeners to other components that are fired once', funct assert.equal(listenerFired, 1, 'listener was executed once'); comp2.trigger('test-event'); assert.equal(listenerFired, 1, 'listener was executed only once'); + + comp1.dispose(); + comp2.dispose(); }); QUnit.test('should add listeners to other element and remove them', function(assert) { @@ -491,6 +522,8 @@ QUnit.test('should add listeners to other element and remove them', function(ass } Events.trigger(el, 'dispose'); assert.ok(true, 'this component removed dispose listeners from other element'); + + comp1.dispose(); }); QUnit.test('should add listeners to other components that are fired once', function(assert) { @@ -509,6 +542,8 @@ QUnit.test('should add listeners to other components that are fired once', funct assert.equal(listenerFired, 1, 'listener was executed once'); Events.trigger(el, 'test-event'); assert.equal(listenerFired, 1, 'listener was executed only once'); + + comp1.dispose(); }); QUnit.test('should trigger a listener when ready', function(assert) { @@ -549,6 +584,8 @@ QUnit.test('should trigger a listener when ready', function(assert) { assert.ok(!initListenerFired, 'init listener should be removed'); assert.ok(!methodListenerFired, 'method listener should be removed'); assert.ok(!syncListenerFired, 'sync listener should be removed'); + + comp.dispose(); }); QUnit.test('should not retrigger a listener when the listener calls triggerReady', function(assert) { @@ -573,6 +610,8 @@ QUnit.test('should not retrigger a listener when the listener calls triggerReady this.clock.tick(100); assert.equal(timesCalled, 1, 'triggerReady from inside a ready handler does not result in an infinite loop'); + + comp.dispose(); }); QUnit.test('should add and remove a CSS class', function(assert) { @@ -586,6 +625,8 @@ QUnit.test('should add and remove a CSS class', function(assert) { assert.ok(comp.el().className.indexOf('test-class') !== -1); comp.toggleClass('test-class'); assert.ok(comp.el().className.indexOf('test-class') === -1); + + comp.dispose(); }); QUnit.test('should show and hide an element', function(assert) { @@ -595,6 +636,8 @@ QUnit.test('should show and hide an element', function(assert) { assert.ok(comp.hasClass('vjs-hidden') === true); comp.show(); assert.ok(comp.hasClass('vjs-hidden') === false); + + comp.dispose(); }); QUnit.test('dimension() should treat NaN and null as zero', function(assert) { @@ -624,6 +667,8 @@ QUnit.test('dimension() should treat NaN and null as zero', function(assert) { newWidth = comp.dimension('width', undefined); assert.equal(newWidth, width, 'we did not set the width with undefined'); + + comp.dispose(); }); QUnit.test('should change the width and height of a component', function(assert) { @@ -654,6 +699,8 @@ QUnit.test('should change the width and height of a component', function(assert) comp.height('auto'); assert.ok(comp.width() === 1000, 'forced width was removed'); assert.ok(comp.height() === 0, 'forced height was removed'); + + comp.dispose(); }); QUnit.test('should get the computed dimensions', function(assert) { @@ -683,6 +730,7 @@ QUnit.test('should get the computed dimensions', function(assert) { assert.equal(comp.currentDimensions().width + 'px', computedWidth, 'matches computed width'); assert.equal(comp.currentDimensions().height + 'px', computedHeight, 'matches computed width'); + comp.dispose(); }); QUnit.test('should use a defined content el for appending children', function(assert) { @@ -711,6 +759,8 @@ QUnit.test('should use a defined content el for appending children', function(as assert.ok(comp.el().childNodes[0].id === 'contentEl', 'Content El should still exist'); assert.ok(comp.el().childNodes[0].childNodes[0] !== child.el(), 'Child el should be removed.'); + + comp.dispose(); }); QUnit.test('should emit a tap event', function(assert) { @@ -769,6 +819,7 @@ QUnit.test('should emit a tap event', function(assert) { // Reset to orignial value browser.TOUCH_ENABLED = origTouch; + comp.dispose(); }); QUnit.test('should provide timeout methods that automatically get cleared on component disposal', function(assert) { @@ -919,6 +970,8 @@ QUnit.test('$ and $$ functions', function(assert) { assert.strictEqual(comp.$('div'), children[0], '$ defaults to contentEl as scope'); assert.strictEqual(comp.$$('div').length, children.length, '$$ defaults to contentEl as scope'); + + comp.dispose(); }); QUnit.test('should use the stateful mixin', function(assert) { @@ -929,4 +982,6 @@ QUnit.test('should use the stateful mixin', function(assert) { comp.setState({foo: 'bar'}); assert.strictEqual(comp.state.foo, 'bar', 'the component passes a basic stateful test'); + + comp.dispose(); }); diff --git a/test/unit/menu.test.js b/test/unit/menu.test.js index eec84a3ed5..611b5b368d 100644 --- a/test/unit/menu.test.js +++ b/test/unit/menu.test.js @@ -33,6 +33,7 @@ QUnit.test('should place title list item into ul', function(assert) { assert.ok(titleElement.innerHTML === 'TestTitle', 'title element placed in ul'); + menuButton.dispose(); player.dispose(); }); @@ -71,5 +72,6 @@ QUnit.test('clicking should display the menu', function(assert) { assert.equal(menuButton.menu.hasClass('vjs-lock-showing'), true, 'enable() allows clicking to show the menu'); + menuButton.dispose(); player.dispose(); }); diff --git a/test/unit/poster.test.js b/test/unit/poster.test.js index 4dfed56f3e..b9c4544b96 100644 --- a/test/unit/poster.test.js +++ b/test/unit/poster.test.js @@ -44,6 +44,8 @@ QUnit.test('should create and update a poster image', function(assert) { this.mockPlayer.trigger('posterchange'); backgroundImage = posterImage.el().style.backgroundImage; assert.notEqual(backgroundImage.indexOf(this.poster2), -1, 'Background image updated'); + + posterImage.dispose(); }); QUnit.test('should create and update a fallback image in older browsers', function(assert) { @@ -61,6 +63,8 @@ QUnit.test('should create and update a fallback image in older browsers', functi assert.notEqual(posterImage.fallbackImg_.src.indexOf(this.poster2), -1, 'Fallback image updated'); + + posterImage.dispose(); }); QUnit.test('should remove itself from the document flow when there is no poster', function(assert) { @@ -81,6 +85,8 @@ QUnit.test('should remove itself from the document flow when there is no poster' assert.equal(posterImage.hasClass('vjs-hidden'), false, 'Poster image shows again when there is a source'); + + posterImage.dispose(); }); QUnit.test('should hide the poster in the appropriate player states', function(assert) { @@ -106,4 +112,6 @@ QUnit.test('should hide the poster in the appropriate player states', function(a assert.equal(TestHelpers.getComputedStyle(el, 'display'), 'block', 'The poster continues to show when playing audio'); + + posterImage.dispose(); }); diff --git a/test/unit/tracks/audio-tracks.test.js b/test/unit/tracks/audio-tracks.test.js index 5c55e74b11..d5281d242d 100644 --- a/test/unit/tracks/audio-tracks.test.js +++ b/test/unit/tracks/audio-tracks.test.js @@ -72,6 +72,7 @@ QUnit.test('listen to remove and add track events in native audio tracks', funct Html5.TEST_VID = oldTestVid; Html5.prototype.audioTracks = oldAudioTracks; + html.dispose(); }); QUnit.test('html5 tech supports native audio tracks if the video supports it', function(assert) { diff --git a/test/unit/tracks/text-tracks.test.js b/test/unit/tracks/text-tracks.test.js index 00199a9239..7817fc8771 100644 --- a/test/unit/tracks/text-tracks.test.js +++ b/test/unit/tracks/text-tracks.test.js @@ -121,6 +121,8 @@ QUnit.test('listen to remove and add track events in native text tracks', functi Html5.TEST_VID = oldTestVid; Html5.prototype.textTracks = oldTextTracks; + + html.dispose(); }); QUnit.test('update texttrack buttons on removetrack or addtrack', function(assert) { @@ -278,6 +280,8 @@ if (Html5.supportsNativeTextTracks()) { emulatedTt.on('removetrack', function() { assert.equal(emulatedTt.length, tt.length, 'we have matching tracks length'); assert.equal(emulatedTt.length, 0, 'we have no more text tracks'); + + html.dispose(); done(); }); }); From a0630760d7b2e948034aaac2c11978d41e1f43d4 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 16:17:05 -0500 Subject: [PATCH 09/32] add .off to fake player in poster image tests --- test/unit/poster.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/poster.test.js b/test/unit/poster.test.js index b9c4544b96..7511731c9b 100644 --- a/test/unit/poster.test.js +++ b/test/unit/poster.test.js @@ -18,6 +18,7 @@ QUnit.module('PosterImage', { return this.poster_; }, handler_: null, + off() {}, on(type, handler) { this.handler_ = handler; }, From 818a57412669abbc2d53f473a785da2ef1b11f37 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 16:23:51 -0500 Subject: [PATCH 10/32] add removeEventListener to tracks tests --- test/unit/tracks/audio-tracks.test.js | 1 + test/unit/tracks/text-tracks.test.js | 3 +++ test/unit/tracks/video-tracks.test.js | 1 + 3 files changed, 5 insertions(+) diff --git a/test/unit/tracks/audio-tracks.test.js b/test/unit/tracks/audio-tracks.test.js index d5281d242d..0bf827e578 100644 --- a/test/unit/tracks/audio-tracks.test.js +++ b/test/unit/tracks/audio-tracks.test.js @@ -33,6 +33,7 @@ QUnit.test('listen to remove and add track events in native audio tracks', funct Html5.prototype.audioTracks = function() { return { + removeEventListener() {}, addEventListener(type, handler) { events[type] = true; } diff --git a/test/unit/tracks/text-tracks.test.js b/test/unit/tracks/text-tracks.test.js index 7817fc8771..c7a4ca0791 100644 --- a/test/unit/tracks/text-tracks.test.js +++ b/test/unit/tracks/text-tracks.test.js @@ -82,6 +82,7 @@ QUnit.test('listen to remove and add track events in native text tracks', functi Html5.prototype.textTracks = function() { return { + removeEventListener() {}, addEventListener(type, handler) { events[type] = true; } @@ -323,6 +324,8 @@ QUnit.test('should check for text track changes when emulating text tracks', fun }); tech.emulateTextTracks(); assert.equal(numTextTrackChanges, 1, 'we got a texttrackchange event'); + + tech.dispose(); }); QUnit.test('removes cuechange event when text track is hidden for emulated tracks', function(assert) { diff --git a/test/unit/tracks/video-tracks.test.js b/test/unit/tracks/video-tracks.test.js index fc2c35b3ae..e1d8121210 100644 --- a/test/unit/tracks/video-tracks.test.js +++ b/test/unit/tracks/video-tracks.test.js @@ -33,6 +33,7 @@ QUnit.test('listen to remove and add track events in native video tracks', funct Html5.prototype.videoTracks = function() { return { + removeEventListener() {}, addEventListener(type, handler) { events[type] = true; } From 8b1cee1bb4cf7429e29ebdecd243cd15f99129b3 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 17:04:39 -0500 Subject: [PATCH 11/32] nullcheck contentWindow --- src/js/resize-manager.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index d28363cd0a..f84ca74da0 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -22,7 +22,9 @@ class ResizeManager extends Component { } else { this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); this.loadListener_ = () => { - this.el_.contentWindow.addEventListener('resize', this.iframeResizeHandler_); + if (this.el_.contentWindow) { + this.el_.contentWindow.addEventListener('resize', this.iframeResizeHandler_); + } this.el_.removeEventListener('load', this.loadListener_); }; From e0400e5d141badec24391c2704b7bd64811988bd Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 17:26:53 -0500 Subject: [PATCH 12/32] use vjs event helpers --- src/js/resize-manager.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index f84ca74da0..5aa7088b1c 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -23,12 +23,12 @@ class ResizeManager extends Component { this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); this.loadListener_ = () => { if (this.el_.contentWindow) { - this.el_.contentWindow.addEventListener('resize', this.iframeResizeHandler_); + this.on(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); } - this.el_.removeEventListener('load', this.loadListener_); + this.off('load', this.loadListener_); }; - this.el_.addEventListener('load', this.loadListener_); + this.on('load', this.loadListener_); } } @@ -53,11 +53,11 @@ class ResizeManager extends Component { } if (this.iframeResizeHandler_ && this.el_.contentWindow) { - this.el_.contentWindow.removeEventListener('resize', this.iframeResizeHandler_); + this.off(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); } if (this.loadListener_) { - this.el_.removeEventListener('load', this.loadListener_); + this.off('load', this.loadListener_); } this.resizeObserver = null; From e66acb1b40a1c18cb48e1244016e3fcb785d6132 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 17:41:49 -0500 Subject: [PATCH 13/32] can't delegate, use actual event helpers --- src/js/resize-manager.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 5aa7088b1c..9301db4783 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -3,6 +3,7 @@ */ import window from 'global/window'; import * as Fn from './utils/fn.js'; +import * as Events from './utils/events.js'; import Component from './component.js'; const RESIZE_OBSERVER_AVAILABLE = window.ResizeObserver; @@ -23,7 +24,7 @@ class ResizeManager extends Component { this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); this.loadListener_ = () => { if (this.el_.contentWindow) { - this.on(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); + Events.on(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); } this.off('load', this.loadListener_); }; @@ -53,7 +54,7 @@ class ResizeManager extends Component { } if (this.iframeResizeHandler_ && this.el_.contentWindow) { - this.off(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); + Events.off(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); } if (this.loadListener_) { From a2494d525400ed0a59ed6b2c6fcd00573a1efbea Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 18:00:34 -0500 Subject: [PATCH 14/32] disable resize manager to IE8 --- src/js/player.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index f16ef51f9e..21b8e91a94 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -3450,8 +3450,7 @@ Player.prototype.options_ = { 'bigPlayButton', 'controlBar', 'errorDisplay', - 'textTrackSettings', - 'resizeManager' + 'textTrackSettings' ], language: navigator && (navigator.languages && navigator.languages[0] || navigator.userLanguage || navigator.language) || 'en', @@ -3463,6 +3462,10 @@ Player.prototype.options_ = { notSupportedMessage: 'No compatible source was found for this media.' }; +if (!browser.IS_IE8) { + Player.prototype.options_.push('resizeManager'); +} + [ /** * Returns whether or not the player is in the "ended" state. From 4fed93944e5839718a41e4c60a3e89eaddbff784 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 9 Jan 2018 18:06:29 -0500 Subject: [PATCH 15/32] push to children --- src/js/player.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/player.js b/src/js/player.js index 21b8e91a94..a2490e0b1c 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -3463,7 +3463,7 @@ Player.prototype.options_ = { }; if (!browser.IS_IE8) { - Player.prototype.options_.push('resizeManager'); + Player.prototype.options_.children.push('resizeManager'); } [ From 69ad100e8911287b50d281f62bf4919cb1bff81e Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 14:00:31 -0500 Subject: [PATCH 16/32] allow a ResizeObserver polyfill to be passed in. --- src/js/resize-manager.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 9301db4783..fef3b2703e 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -6,18 +6,17 @@ import * as Fn from './utils/fn.js'; import * as Events from './utils/events.js'; import Component from './component.js'; -const RESIZE_OBSERVER_AVAILABLE = window.ResizeObserver; - class ResizeManager extends Component { constructor(player, options) { super(player, options); + this.ResizeObserver = options.ResizeObserver || window.ResizeObserver; this.iframeResizeHandler_ = null; this.loadListener_ = null; this.resizeObserver = null; - if (RESIZE_OBSERVER_AVAILABLE) { - this.resizeObserver = new window.ResizeObserver(() => this.resizeHandler()); + if (this.ResizeObserver) { + this.resizeObserver = new this.ResizeObserver(() => this.resizeHandler()); this.resizeObserver.observe(player.el()); } else { @@ -34,7 +33,7 @@ class ResizeManager extends Component { } createEl() { - if (RESIZE_OBSERVER_AVAILABLE) { + if (this.ResizeObserver) { return; } From 157adcf0533b334b5a7fef999c8aafc160e81327 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 16:02:36 -0500 Subject: [PATCH 17/32] test RM, can't test iframe because async --- test/unit/resize-manager.test.js | 73 ++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 test/unit/resize-manager.test.js diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js new file mode 100644 index 0000000000..e61b6a511b --- /dev/null +++ b/test/unit/resize-manager.test.js @@ -0,0 +1,73 @@ +/* eslint-env qunit */ +import ResizeManager from '../../src/js/resize-manager.js'; +import TestHelpers from './test-helpers.js'; +import * as browser from '../../src/js/utils/browser.js'; + +if (!browser.IS_IE8) { + +QUnit.module('ResizeManager', { + beforeEach() { + this.player = TestHelpers.makePlayer(); + }, + afterEach() { + this.player.dispose(); + } +}); + +QUnit.test('ResizeManager creates an iframe if ResizeObserver is not available', function(assert) { + const rm = new ResizeManager(this.player, {ResizeObserver: null}); + + assert.equal(rm.el().tagName.toLowerCase(), 'iframe', 'we got an iframe'); + + rm.dispose(); +}); + +QUnit.test('ResizeManager uses the ResizeObserver, if given', function(assert) { + let roCreated = false; + let observeCalled = false; + class MyResizeObserver { + constructor(fn) { + roCreated = true; + this.observer = fn; + } + + observe(el) { + observeCalled = true; + this.el = el + } + }; + + const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); + + assert.ok(roCreated, 'we intantiated the RO that was passed'); + assert.ok(observeCalled, 'we observed the RO'); + assert.equal(rm.resizeObserver.el, this.player.el(), 'we observed the player el'); + + rm.dispose(); +}); + +QUnit.test('ResizeManager triggers `playerresize` when the observer method is called', function(assert) { + class MyResizeObserver { + constructor(fn) { + this.observer = fn; + } + + observe(el) { + this.el = el + } + }; + + let playerresizeCalled = 0; + const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); + + this.player.on('playerresize', function() { + playerresizeCalled++; + }); + rm.resizeObserver.observer(); + + assert.equal(playerresizeCalled, 1, 'playerresize was triggered'); + + rm.dispose(); +}); + +} From b9c5967ec2fe3b89a0051e2d6eb8836bf2af4cad Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 16:21:48 -0500 Subject: [PATCH 18/32] lint --- test/unit/resize-manager.test.js | 103 ++++++++++++++++--------------- 1 file changed, 52 insertions(+), 51 deletions(-) diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js index e61b6a511b..d1545df1ff 100644 --- a/test/unit/resize-manager.test.js +++ b/test/unit/resize-manager.test.js @@ -5,69 +5,70 @@ import * as browser from '../../src/js/utils/browser.js'; if (!browser.IS_IE8) { -QUnit.module('ResizeManager', { - beforeEach() { - this.player = TestHelpers.makePlayer(); - }, - afterEach() { - this.player.dispose(); - } -}); - -QUnit.test('ResizeManager creates an iframe if ResizeObserver is not available', function(assert) { - const rm = new ResizeManager(this.player, {ResizeObserver: null}); - - assert.equal(rm.el().tagName.toLowerCase(), 'iframe', 'we got an iframe'); - - rm.dispose(); -}); - -QUnit.test('ResizeManager uses the ResizeObserver, if given', function(assert) { - let roCreated = false; - let observeCalled = false; - class MyResizeObserver { - constructor(fn) { - roCreated = true; - this.observer = fn; + QUnit.module('ResizeManager', { + beforeEach() { + this.player = TestHelpers.makePlayer(); + }, + afterEach() { + this.player.dispose(); } + }); - observe(el) { - observeCalled = true; - this.el = el - } - }; + QUnit.test('ResizeManager creates an iframe if ResizeObserver is not available', function(assert) { + const rm = new ResizeManager(this.player, {ResizeObserver: null}); + + assert.equal(rm.el().tagName.toLowerCase(), 'iframe', 'we got an iframe'); - const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); + rm.dispose(); + }); - assert.ok(roCreated, 'we intantiated the RO that was passed'); - assert.ok(observeCalled, 'we observed the RO'); - assert.equal(rm.resizeObserver.el, this.player.el(), 'we observed the player el'); + QUnit.test('ResizeManager uses the ResizeObserver, if given', function(assert) { + let roCreated = false; + let observeCalled = false; - rm.dispose(); -}); + class MyResizeObserver { + constructor(fn) { + roCreated = true; + this.observer = fn; + } -QUnit.test('ResizeManager triggers `playerresize` when the observer method is called', function(assert) { - class MyResizeObserver { - constructor(fn) { - this.observer = fn; + observe(el) { + observeCalled = true; + this.el = el; + } } - observe(el) { - this.el = el - } - }; + const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); - let playerresizeCalled = 0; - const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); + assert.ok(roCreated, 'we intantiated the RO that was passed'); + assert.ok(observeCalled, 'we observed the RO'); + assert.equal(rm.resizeObserver.el, this.player.el(), 'we observed the player el'); - this.player.on('playerresize', function() { - playerresizeCalled++; + rm.dispose(); }); - rm.resizeObserver.observer(); - assert.equal(playerresizeCalled, 1, 'playerresize was triggered'); + QUnit.test('ResizeManager triggers `playerresize` when the observer method is called', function(assert) { + class MyResizeObserver { + constructor(fn) { + this.observer = fn; + } - rm.dispose(); -}); + observe(el) { + this.el = el; + } + } + + let playerresizeCalled = 0; + const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); + + this.player.on('playerresize', function() { + playerresizeCalled++; + }); + rm.resizeObserver.observer(); + + assert.equal(playerresizeCalled, 1, 'playerresize was triggered'); + + rm.dispose(); + }); } From 18854a3753bfc83b7eb063c7c42eab0c57293f28 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 16:39:37 -0500 Subject: [PATCH 19/32] point at right player --- src/js/resize-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index fef3b2703e..3c82e85df4 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -48,7 +48,7 @@ class ResizeManager extends Component { dispose() { if (this.resizeObserver) { - this.resizeObserver.unobserve(this.player.el()); + this.resizeObserver.unobserve(this.player_.el()); this.resizeObserver.disconnect(); } From 6b7c278675168fe567a8ce6bc5beec3f6c23ec37 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 16:42:18 -0500 Subject: [PATCH 20/32] update tests --- test/unit/resize-manager.test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js index d1545df1ff..eb03c569ec 100644 --- a/test/unit/resize-manager.test.js +++ b/test/unit/resize-manager.test.js @@ -25,6 +25,9 @@ if (!browser.IS_IE8) { QUnit.test('ResizeManager uses the ResizeObserver, if given', function(assert) { let roCreated = false; let observeCalled = false; + let unobserveCalled = false; + let disconnectCalled = false; + let sameEl = false; class MyResizeObserver { constructor(fn) { @@ -36,6 +39,15 @@ if (!browser.IS_IE8) { observeCalled = true; this.el = el; } + + unobserve(el) { + unobserveCalled = true; + sameEl = this.el === el; + } + + disconnect() { + disconnectCalled = true; + } } const rm = new ResizeManager(this.player, {ResizeObserver: MyResizeObserver}); @@ -45,6 +57,10 @@ if (!browser.IS_IE8) { assert.equal(rm.resizeObserver.el, this.player.el(), 'we observed the player el'); rm.dispose(); + + assert.ok(unobserveCalled, 'we unobserve when disposed'); + assert.ok(sameEl, 'we unobserve the same el as we observed'); + assert.ok(disconnectCalled, 'we disconnected when disposed'); }); QUnit.test('ResizeManager triggers `playerresize` when the observer method is called', function(assert) { @@ -56,6 +72,12 @@ if (!browser.IS_IE8) { observe(el) { this.el = el; } + + unobserve(el) { + } + + disconnect() { + } } let playerresizeCalled = 0; From 902bfde225a0c9b5b3ee95840b7218f411f833cb Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:06:24 -0500 Subject: [PATCH 21/32] add debounce inspired by lodash and underscore --- src/js/utils/fn.js | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/js/utils/fn.js b/src/js/utils/fn.js index 535b8b1bae..979add1c5a 100644 --- a/src/js/utils/fn.js +++ b/src/js/utils/fn.js @@ -68,3 +68,58 @@ export const throttle = function(fn, wait) { return throttled; }; + +/** + * Creates a debounced function that delays invoking `func` until after `wait` + * milliseconds have elapsed since the last time the debounced function was + * invoked. + * + * Inspired by lodash and underscore implementations. + * + * @param {Function} func + * The function to wrap with debounce behavior. + * + * @param {number} wait + * The number of milliseconds to wait after the last invocation. + * + * @param {boolean} [immediate] + * Whether or not to invoke the function immediately upon creation. + * + * @param {Object} [context=window] + * The "context" in which the debounced function should debounce. For + * example, if this function should be tied to a Video.js player, + * the player can be passed here. Alternatively, defaults to the + * global `window` object. + * + * @return {Function} + * A debounced function. + */ +export const debounce = function(func, wait, immediate, context = window) { + let timeout; + + + /* eslint-disable consistent-this */ + return function() { + const self = this; + const args = arguments; + + + let later = function() { + timeout = null; + later = null; + if (!immediate) { + func.apply(self, args); + } + }; + + + if (!timeout && immediate) { + func.apply(self, args); + } + + + context.clearTimeout(timeout); + timeout = context.setTimeout(later, wait); + }; + /* eslint-enable consistent-this */ +} From 4fa8e31b3017d697e223980641e0b59b4c628dd3 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:09:43 -0500 Subject: [PATCH 22/32] debounce handlers and don't create el if not necessary --- src/js/resize-manager.js | 44 +++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 3c82e85df4..1bd7ca414e 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -2,28 +2,37 @@ * @file resize-manager.js */ import window from 'global/window'; -import * as Fn from './utils/fn.js'; +import { debounce } from './utils/fn.js'; import * as Events from './utils/events.js'; +import mergeOptions from './utils/merge-options.js'; import Component from './component.js'; class ResizeManager extends Component { constructor(player, options) { - super(player, options); + let RESIZE_OBSERVER_AVAILABLE = options.ResizeObserver || window.ResizeObserver; + + if (options.ResizeObserver === null) { + RESIZE_OBSERVER_AVAILABLE = false; + } + + // Only create an element when ResizeObserver isn't available + const options_ = mergeOptions({createEl: !RESIZE_OBSERVER_AVAILABLE}, options); + + super(player, options_); this.ResizeObserver = options.ResizeObserver || window.ResizeObserver; - this.iframeResizeHandler_ = null; + this.debouncedHandler_ = debounce(() => { this.resizeHandler() }, 100); this.loadListener_ = null; - this.resizeObserver = null; + this.resizeObserver_ = null; - if (this.ResizeObserver) { - this.resizeObserver = new this.ResizeObserver(() => this.resizeHandler()); - this.resizeObserver.observe(player.el()); + if (RESIZE_OBSERVER_AVAILABLE) { + this.resizeObserver_ = new this.ResizeObserver(this.debouncedHandler_); + this.resizeObserver_.observe(player.el()); } else { - this.iframeResizeHandler_ = Fn.throttle(() => this.resizeHandler(), 50); this.loadListener_ = () => { if (this.el_.contentWindow) { - Events.on(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); + Events.on(this.el_.contentWindow, 'resize', this.debouncedHandler_); } this.off('load', this.loadListener_); }; @@ -33,10 +42,6 @@ class ResizeManager extends Component { } createEl() { - if (this.ResizeObserver) { - return; - } - return super.createEl('iframe', { className: 'vjs-resize-manager' }); @@ -47,21 +52,22 @@ class ResizeManager extends Component { } dispose() { - if (this.resizeObserver) { - this.resizeObserver.unobserve(this.player_.el()); - this.resizeObserver.disconnect(); + if (this.resizeObserver_) { + this.resizeObserver_.unobserve(this.player_.el()); + this.resizeObserver_.disconnect(); } - if (this.iframeResizeHandler_ && this.el_.contentWindow) { - Events.off(this.el_.contentWindow, 'resize', this.iframeResizeHandler_); + if (this.debouncedHandler_ && this.el_.contentWindow) { + Events.off(this.el_.contentWindow, 'resize', this.debouncedHandler_); } if (this.loadListener_) { this.off('load', this.loadListener_); } + this.ResizeObserver = null; this.resizeObserver = null; - this.iframeResizeHandler_ = null; + this.debouncedHandler_ = null; this.loadListener_ = null; } From 1ffd0aab7f14bf795f2e856bf46f7eefba2e5737 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:13:17 -0500 Subject: [PATCH 23/32] fixup tests --- src/js/resize-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 1bd7ca414e..cd6205a1f6 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -57,7 +57,7 @@ class ResizeManager extends Component { this.resizeObserver_.disconnect(); } - if (this.debouncedHandler_ && this.el_.contentWindow) { + if (this.el_ && this.el_.contentWindow) { Events.off(this.el_.contentWindow, 'resize', this.debouncedHandler_); } From a8f6ebb6555b6d55865e9ef8b9c1e315fae6d87e Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:23:51 -0500 Subject: [PATCH 24/32] dispose debounce with player, use fake timers in test --- src/js/resize-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index cd6205a1f6..1048673dad 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -21,7 +21,7 @@ class ResizeManager extends Component { super(player, options_); this.ResizeObserver = options.ResizeObserver || window.ResizeObserver; - this.debouncedHandler_ = debounce(() => { this.resizeHandler() }, 100); + this.debouncedHandler_ = debounce(() => { this.resizeHandler() }, 100, false, player); this.loadListener_ = null; this.resizeObserver_ = null; From fec334bc939e4cbbf913697e94cb49d67d8849fe Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:24:42 -0500 Subject: [PATCH 25/32] sinon fake timers --- test/unit/resize-manager.test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js index eb03c569ec..6ddc471b4b 100644 --- a/test/unit/resize-manager.test.js +++ b/test/unit/resize-manager.test.js @@ -7,10 +7,12 @@ if (!browser.IS_IE8) { QUnit.module('ResizeManager', { beforeEach() { + this.clock = sinon.useFakeTimers(); this.player = TestHelpers.makePlayer(); }, afterEach() { this.player.dispose(); + this.clock.restore(); } }); @@ -54,7 +56,7 @@ if (!browser.IS_IE8) { assert.ok(roCreated, 'we intantiated the RO that was passed'); assert.ok(observeCalled, 'we observed the RO'); - assert.equal(rm.resizeObserver.el, this.player.el(), 'we observed the player el'); + assert.equal(rm.resizeObserver_.el, this.player.el(), 'we observed the player el'); rm.dispose(); @@ -86,7 +88,9 @@ if (!browser.IS_IE8) { this.player.on('playerresize', function() { playerresizeCalled++; }); - rm.resizeObserver.observer(); + rm.resizeObserver_.observer(); + + this.clock.tick(100); assert.equal(playerresizeCalled, 1, 'playerresize was triggered'); From a742ad405dd31a1af460a742bb18e6191a474d41 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 10 Jan 2018 17:28:09 -0500 Subject: [PATCH 26/32] linter --- src/js/resize-manager.js | 4 +++- src/js/utils/fn.js | 7 ++----- test/unit/resize-manager.test.js | 1 + 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 1048673dad..51d6b9d728 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -21,9 +21,11 @@ class ResizeManager extends Component { super(player, options_); this.ResizeObserver = options.ResizeObserver || window.ResizeObserver; - this.debouncedHandler_ = debounce(() => { this.resizeHandler() }, 100, false, player); this.loadListener_ = null; this.resizeObserver_ = null; + this.debouncedHandler_ = debounce(() => { + this.resizeHandler(); + }, 100, false, player); if (RESIZE_OBSERVER_AVAILABLE) { this.resizeObserver_ = new this.ResizeObserver(this.debouncedHandler_); diff --git a/src/js/utils/fn.js b/src/js/utils/fn.js index 979add1c5a..63e10087d1 100644 --- a/src/js/utils/fn.js +++ b/src/js/utils/fn.js @@ -3,6 +3,7 @@ * @module fn */ import { newGUID } from './guid.js'; +import window from 'global/window'; /** * Bind (a.k.a proxy or Context). A simple method for changing the context of a function @@ -97,13 +98,11 @@ export const throttle = function(fn, wait) { export const debounce = function(func, wait, immediate, context = window) { let timeout; - /* eslint-disable consistent-this */ return function() { const self = this; const args = arguments; - let later = function() { timeout = null; later = null; @@ -112,14 +111,12 @@ export const debounce = function(func, wait, immediate, context = window) { } }; - if (!timeout && immediate) { func.apply(self, args); } - context.clearTimeout(timeout); timeout = context.setTimeout(later, wait); }; /* eslint-enable consistent-this */ -} +}; diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js index 6ddc471b4b..684801d929 100644 --- a/test/unit/resize-manager.test.js +++ b/test/unit/resize-manager.test.js @@ -2,6 +2,7 @@ import ResizeManager from '../../src/js/resize-manager.js'; import TestHelpers from './test-helpers.js'; import * as browser from '../../src/js/utils/browser.js'; +import sinon from 'sinon'; if (!browser.IS_IE8) { From 5cb16a3b9299cffabc04f231430541397f7541b2 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 17 Jan 2018 14:58:31 -0500 Subject: [PATCH 27/32] add jsdoc to resize manager --- src/js/resize-manager.js | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 51d6b9d728..e1f080de87 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -7,7 +7,37 @@ import * as Events from './utils/events.js'; import mergeOptions from './utils/merge-options.js'; import Component from './component.js'; +/** + * A Resize Manager. It is in charge of trigger `playerresize` on the player on the right conditions. + * + * It'll either create an iframe and use a debounced resize handler on it or use the new {@link https://wicg.github.io/ResizeObserver/|ResizeObserver}. + * + * If the ResizeObserver is available natively, it will be used. A polyfill can be passed in as an option. + * If a `playerresize` event is not needed, the ResizeManager component can be removed from the player, see the example below. + * @example How to disable the resize manager + * const player = videojs('#vid', { + * resizeManager: false + * }); + * + * @see {@link https://wicg.github.io/ResizeObserver/|ResizeObserver specification} + * + * @extends Component + */ class ResizeManager extends Component { + + /** + * Create the ResizeMAnager. + * + * @param {Object} player + * The `Player` that this class should be attached to. + * + * @param {Object} [options] + * The key/value store of ResizeManager options. + * + * @param {Object} [options.ResizeObserver] + * A polyfill for ResizeObserver can be passed in here. + * If this is set to null it will ignore the native ResizeObserver and fall back to the iframe fallback. + */ constructor(player, options) { let RESIZE_OBSERVER_AVAILABLE = options.ResizeObserver || window.ResizeObserver; @@ -49,7 +79,18 @@ class ResizeManager extends Component { }); } + /** + * Called when a resize is triggered on the iframe or a resize is observed via the ResizeObserver + * + * @fires Player#playerresize + */ resizeHandler() { + /** + * Called when the player size has changed + * + * @event Player#playerresize + * @type {EventTarget~Event} + */ this.player_.trigger('playerresize'); } From e8912609918380b00cb16992f59b3d80d4fa3fef Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 17 Jan 2018 14:58:46 -0500 Subject: [PATCH 28/32] update glob for remark --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index c43e042a29..039957d18b 100644 --- a/package.json +++ b/package.json @@ -35,8 +35,8 @@ "docs:api": "jsdoc -c .jsdoc.json", "postdocs:api": "node ./build/fix-api-docs.js", "netlify": "babel-node ./build/netlify-docs.js", - "docs:lint": "remark -- './**/*.md'", - "docs:fix": "remark --output -- './**/*.md'", + "docs:lint": "remark -- './{,!(node_modules)/**/}!(CHANGELOG)*.md'", + "docs:fix": "remark --output -- './{,!(node_modules)/**/}!(CHANGELOG)*.md'", "babel": "babel src/js -d es5", "prepublish": "not-in-install && run-p build || in-install", "publish": "node build/gh-release.js", From 062d31f664a45f1f23663aea28c387cdce02b72e Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 17 Jan 2018 15:09:19 -0500 Subject: [PATCH 29/32] add ResizeManager to components --- docs/guides/components.md | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/guides/components.md b/docs/guides/components.md index 082327c820..2454b5af7f 100644 --- a/docs/guides/components.md +++ b/docs/guides/components.md @@ -312,7 +312,8 @@ Player │ ├── AudioTrackButton (hidden, unless there are relevant tracks) │ └── FullscreenToggle ├── ErrorDisplay (hidden, until there is an error) -└── TextTrackSettings +├── TextTrackSettings +└── ResizeManager (hidden) ``` ## Specific Component Details @@ -338,3 +339,34 @@ let player = videojs('myplayer', { The text track settings component is only available when using emulated text tracks. [api]: http://docs.videojs.com/Component.html + +### Resize Manager + +This new component is in charge of triggering a `playerresize` event when the player size changed. +It uses the ResizeObserver if available or a polyfill was provided. It has no element when using the ResizeObserver. +If a ResizeObserver is not available, it will fallback to an iframe element and listen to its resize event via a debounced handler. + +A ResizeObserver polyfill can be passed in like so: +```js +var player = videojs('myplayer', { + resizeManager: { + ResizeObserver: ResizeObserverPoylfill + } +}); +``` + +To force using the iframe fallback, pass in `null` as the `ResizeObserver`: +```js +var player = videojs('myplayer', { + resizeManager: { + ResizeObserver: null + } +}); +``` + +The ResizeManager can also just be disabled like so: +```js +var player = videojs('myplayer', { + resizeManager: false +}); +``` From 93449c0a67360dbd1c902d68900465603c47a7df Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 17 Jan 2018 15:10:33 -0500 Subject: [PATCH 30/32] remark fix components --- docs/guides/components.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/guides/components.md b/docs/guides/components.md index 2454b5af7f..c48849c394 100644 --- a/docs/guides/components.md +++ b/docs/guides/components.md @@ -18,6 +18,7 @@ The architecture of the Video.js player is centered around components. The `Play * [Specific Component Details](#specific-component-details) * [Volume Panel](#volume-panel) * [Text Track Settings](#text-track-settings) + * [Resize Manager](#resize-manager) ## What is a Component? @@ -347,6 +348,7 @@ It uses the ResizeObserver if available or a polyfill was provided. It has no el If a ResizeObserver is not available, it will fallback to an iframe element and listen to its resize event via a debounced handler. A ResizeObserver polyfill can be passed in like so: + ```js var player = videojs('myplayer', { resizeManager: { @@ -356,6 +358,7 @@ var player = videojs('myplayer', { ``` To force using the iframe fallback, pass in `null` as the `ResizeObserver`: + ```js var player = videojs('myplayer', { resizeManager: { @@ -365,6 +368,7 @@ var player = videojs('myplayer', { ``` The ResizeManager can also just be disabled like so: + ```js var player = videojs('myplayer', { resizeManager: false From 18d354ba832f207ed11ec46075966fd1e7fb30f1 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 17 Jan 2018 15:12:58 -0500 Subject: [PATCH 31/32] code review comments --- src/js/resize-manager.js | 1 + test/unit/resize-manager.test.js | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index e1f080de87..125948e65f 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -41,6 +41,7 @@ class ResizeManager extends Component { constructor(player, options) { let RESIZE_OBSERVER_AVAILABLE = options.ResizeObserver || window.ResizeObserver; + // if `null` was passed, we want to disable the ResizeObserver if (options.ResizeObserver === null) { RESIZE_OBSERVER_AVAILABLE = false; } diff --git a/test/unit/resize-manager.test.js b/test/unit/resize-manager.test.js index 684801d929..689a0727ae 100644 --- a/test/unit/resize-manager.test.js +++ b/test/unit/resize-manager.test.js @@ -35,7 +35,6 @@ if (!browser.IS_IE8) { class MyResizeObserver { constructor(fn) { roCreated = true; - this.observer = fn; } observe(el) { @@ -67,9 +66,11 @@ if (!browser.IS_IE8) { }); QUnit.test('ResizeManager triggers `playerresize` when the observer method is called', function(assert) { + let observer; + class MyResizeObserver { constructor(fn) { - this.observer = fn; + observer = fn; } observe(el) { @@ -89,7 +90,7 @@ if (!browser.IS_IE8) { this.player.on('playerresize', function() { playerresizeCalled++; }); - rm.resizeObserver_.observer(); + observer(); this.clock.tick(100); From 304179233a829525050cbe2c197092e8959a6013 Mon Sep 17 00:00:00 2001 From: ldayananda Date: Thu, 18 Jan 2018 13:34:54 -0500 Subject: [PATCH 32/32] small typo corrections --- src/js/resize-manager.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 125948e65f..6be368999f 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -8,7 +8,7 @@ import mergeOptions from './utils/merge-options.js'; import Component from './component.js'; /** - * A Resize Manager. It is in charge of trigger `playerresize` on the player on the right conditions. + * A Resize Manager. It is in charge of triggering `playerresize` on the player in the right conditions. * * It'll either create an iframe and use a debounced resize handler on it or use the new {@link https://wicg.github.io/ResizeObserver/|ResizeObserver}. * @@ -26,7 +26,7 @@ import Component from './component.js'; class ResizeManager extends Component { /** - * Create the ResizeMAnager. + * Create the ResizeManager. * * @param {Object} player * The `Player` that this class should be attached to.