From d524e5755d347ab5f0eb77b1d2496a5e2e71a3ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <34163393+amtins@users.noreply.github.com> Date: Wed, 31 May 2023 16:54:51 +0200 Subject: [PATCH] fix(picture-in-picture-control): hide the component in non-compatible browsers (#7899) --- src/js/control-bar/control-bar.js | 10 +--- .../control-bar/picture-in-picture-toggle.js | 54 +++++++++++++------ test/unit/controls.test.js | 30 ++++++++++- test/unit/player.test.js | 15 ++++-- 4 files changed, 79 insertions(+), 30 deletions(-) diff --git a/src/js/control-bar/control-bar.js b/src/js/control-bar/control-bar.js index f66590d775..cc03fe98ea 100644 --- a/src/js/control-bar/control-bar.js +++ b/src/js/control-bar/control-bar.js @@ -2,7 +2,6 @@ * @file control-bar.js */ import Component from '../component.js'; -import document from 'global/document'; // Required children import './play-toggle.js'; @@ -73,17 +72,10 @@ ControlBar.prototype.options_ = { 'descriptionsButton', 'subsCapsButton', 'audioTrackButton', + 'pictureInPictureToggle', 'fullscreenToggle' ] }; -if ('exitPictureInPicture' in document) { - ControlBar.prototype.options_.children.splice( - ControlBar.prototype.options_.children.length - 1, - 0, - 'pictureInPictureToggle' - ); -} - Component.registerComponent('ControlBar', ControlBar); export default ControlBar; diff --git a/src/js/control-bar/picture-in-picture-toggle.js b/src/js/control-bar/picture-in-picture-toggle.js index a239740194..94ba6fba35 100644 --- a/src/js/control-bar/picture-in-picture-toggle.js +++ b/src/js/control-bar/picture-in-picture-toggle.js @@ -27,23 +27,10 @@ class PictureInPictureToggle extends Button { */ constructor(player, options) { super(player, options); + this.on(player, ['enterpictureinpicture', 'leavepictureinpicture'], (e) => this.handlePictureInPictureChange(e)); this.on(player, ['disablepictureinpicturechanged', 'loadedmetadata'], (e) => this.handlePictureInPictureEnabledChange(e)); - - this.on(player, ['loadedmetadata', 'audioonlymodechange', 'audiopostermodechange'], () => { - // This audio detection will not detect HLS or DASH audio-only streams because there was no reliable way to detect them at the time - const isSourceAudio = player.currentType().substring(0, 5) === 'audio'; - - if (isSourceAudio || player.audioPosterMode() || player.audioOnlyMode()) { - if (player.isInPictureInPicture()) { - player.exitPictureInPicture(); - } - this.hide(); - } else { - this.show(); - } - - }); + this.on(player, ['loadedmetadata', 'audioonlymodechange', 'audiopostermodechange'], () => this.handlePictureInPictureAudioModeChange()); // TODO: Deactivate button on player emptied event. this.disable(); @@ -56,7 +43,30 @@ class PictureInPictureToggle extends Button { * The DOM `className` for this object. */ buildCSSClass() { - return `vjs-picture-in-picture-control ${super.buildCSSClass()}`; + return `vjs-picture-in-picture-control vjs-hidden ${super.buildCSSClass()}`; + } + + /** + * Displays or hides the button depending on the audio mode detection. + * Exits picture-in-picture if it is enabled when switching to audio mode. + */ + handlePictureInPictureAudioModeChange() { + // This audio detection will not detect HLS or DASH audio-only streams because there was no reliable way to detect them at the time + const isSourceAudio = this.player_.currentType().substring(0, 5) === 'audio'; + const isAudioMode = + isSourceAudio || this.player_.audioPosterMode() || this.player_.audioOnlyMode(); + + if (!isAudioMode) { + this.show(); + + return; + } + + if (this.player_.isInPictureInPicture()) { + this.player_.exitPictureInPicture(); + } + + this.hide(); } /** @@ -117,6 +127,18 @@ class PictureInPictureToggle extends Button { } } + /** + * Show the `Component`s element if it is hidden by removing the + * 'vjs-hidden' class name from it only in browsers that support the Picture-in-Picture API. + */ + show() { + // Does not allow to display the pictureInPictureToggle in browsers that do not support the Picture-in-Picture API, e.g. Firefox. + if (typeof document.exitPictureInPicture !== 'function') { + return; + } + + super.show(); + } } /** diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index a771c1637c..ddad47931e 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -293,7 +293,11 @@ QUnit.test('Picture-in-Picture control is hidden when the source is audio', func player.src({src: 'example.mp4', type: 'video/mp4'}); player.trigger('loadedmetadata'); - assert.notOk(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden initially'); + if (document.exitPictureInPicture) { + assert.notOk(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden initially'); + } else { + assert.ok(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is hidden if PiP is not supported'); + } player.src({src: 'example1.mp3', type: 'audio/mp3'}); player.trigger('loadedmetadata'); @@ -318,7 +322,11 @@ QUnit.test('Picture-in-Picture control is displayed if docPiP is enabled', funct player.src({src: 'example.mp4', type: 'video/mp4'}); player.trigger('loadedmetadata'); - assert.notOk(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden'); + if (document.exitPictureInPicture) { + assert.notOk(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden'); + } else { + assert.ok(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is hidden if PiP is not supported'); + } player.dispose(); pictureInPictureToggle.dispose(); @@ -327,6 +335,24 @@ QUnit.test('Picture-in-Picture control is displayed if docPiP is enabled', funct } }); +QUnit.test('Picture-in-Picture control should only be displayed if the browser supports it', function(assert) { + const player = TestHelpers.makePlayer(); + const pictureInPictureToggle = new PictureInPictureToggle(player); + + player.trigger('loadedmetadata'); + + if (document.exitPictureInPicture) { + // Browser that does support PiP + assert.false(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden'); + } else { + // Browser that does not support PiP + assert.true(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is hidden'); + } + + player.dispose(); + pictureInPictureToggle.dispose(); +}); + QUnit.test('Fullscreen control text should be correct when fullscreenchange is triggered', function(assert) { const player = TestHelpers.makePlayer({controlBar: false}); const fullscreentoggle = new FullscreenToggle(player); diff --git a/test/unit/player.test.js b/test/unit/player.test.js index a595646849..502ac9759c 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -3150,6 +3150,7 @@ QUnit.test('audioOnlyMode(true/false) hides/shows video-specific control bar com controlBar.getChild('ChaptersButton').update(); player.trigger('ready'); + player.trigger('loadedmetadata'); player.hasStarted(true); // Show all control bar children @@ -3157,8 +3158,12 @@ QUnit.test('audioOnlyMode(true/false) hides/shows video-specific control bar com const el = controlBar.getChild(child) && controlBar.getChild(child).el_; if (el) { - // Sanity check that component is showing - assert.notEqual(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is initially visible`); + if (!document.exitPictureInPicture && child === 'PictureInPictureToggle') { + assert.equal(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is not visible if PiP is not supported`); + } else { + // Sanity check that component is showing + assert.notEqual(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is initially visible`); + } } }); @@ -3187,7 +3192,11 @@ QUnit.test('audioOnlyMode(true/false) hides/shows video-specific control bar com const el = controlBar.getChild(child) && controlBar.getChild(child).el_; if (el) { - assert.notEqual(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is shown`); + if (!document.exitPictureInPicture && child === 'PictureInPictureToggle') { + assert.equal(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is not visible if PiP is not supported`); + } else { + assert.notEqual(TestHelpers.getComputedStyle(el, 'display'), 'none', `${child} is shown`); + } } }); })