Bug 1666775 - Make sure to hide the new Picture-in-Picture toggle on audio-only <video> elements. r=Gijs
authorMike Conley <mconley@mozilla.com>
Thu, 24 Sep 2020 18:05:39 +0000
changeset 550192 38353040d4e797b4a4d31cf1bc57fb8aa3d75079
parent 550191 047eec5c0b1c317dd8718e8a67bfb4cf1708977c
child 550193 718ec7f40f245dc1f7e3cedc1d90c17081bb1912
push id37809
push userapavel@mozilla.com
push dateFri, 25 Sep 2020 03:37:48 +0000
treeherdermozilla-central@4846ccf88574 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1666775
milestone83.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1666775 - Make sure to hide the new Picture-in-Picture toggle on audio-only <video> elements. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D91303
toolkit/components/pictureinpicture/tests/browser.ini
toolkit/components/pictureinpicture/tests/browser_noToggleOnAudio.js
toolkit/content/widgets/videocontrols.js
--- a/toolkit/components/pictureinpicture/tests/browser.ini
+++ b/toolkit/components/pictureinpicture/tests/browser.ini
@@ -9,16 +9,17 @@ support-files =
   test-page-with-sound.html
   test-pointer-events-none.html
   test-transparent-overlay-1.html
   test-transparent-overlay-2.html
   test-video.mp4
   test-video-cropped.mp4
   test-video-vertical.mp4
   ../../../../dom/media/test/gizmo.mp4
+  ../../../../dom/media/test/owl.mp3
 
 prefs =
   media.videocontrols.picture-in-picture.enabled=true
   media.videocontrols.picture-in-picture.video-toggle.enabled=true
   media.videocontrols.picture-in-picture.video-toggle.testing=true
   media.videocontrols.picture-in-picture.video-toggle.always-show=true
   media.videocontrols.picture-in-picture.video-toggle.has-used=true
 
@@ -30,16 +31,17 @@ prefs =
 [browser_disabledForMediaStreamVideos.js]
 skip-if = os == 'mac' #Bug 1622391
 [browser_durationChange.js]
 [browser_fullscreen.js]
 skip-if = (os == "mac" && debug) #Bug 1566173
 [browser_keyboardShortcut.js]
 [browser_mouseButtonVariation.js]
 skip-if = debug
+[browser_noToggleOnAudio.js]
 [browser_playerControls.js]
 [browser_removeVideoElement.js]
 [browser_rerequestPiP.js]
 [browser_resizeVideo.js]
 skip-if = os == 'linux' # Bug 1594223
 [browser_shortcutsAfterFocus.js]
 [browser_showMessage.js]
 [browser_smallVideoLayout.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/pictureinpicture/tests/browser_noToggleOnAudio.js
@@ -0,0 +1,45 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Tests that if a <video> element only has audio, and no video
+ * frames, that we do not show the toggle.
+ */
+add_task(async function test_no_toggle_on_audio() {
+  await BrowserTestUtils.withNewTab(
+    {
+      gBrowser,
+      url: TEST_ROOT + "owl.mp3",
+    },
+    async browser => {
+      await ensureVideosReady(browser);
+      await SimpleTest.promiseFocus(browser);
+
+      // The media player document we create for owl.mp3 inserts a <video>
+      // element pointed at the .mp3 file, which is what we're trying to
+      // test for. The <video> element does not get an ID created for it
+      // though, so we sneak one in with SpecialPowers.spawn so that we
+      // can use testToggleHelper (which requires an ID).
+      //
+      // testToggleHelper also wants click-event-helper.js loaded in the
+      // document, so we insert that too.
+      const VIDEO_ID = "video-element";
+      const SCRIPT_SRC = "click-event-helper.js";
+      await SpecialPowers.spawn(browser, [VIDEO_ID, SCRIPT_SRC], async function(
+        videoID,
+        scriptSrc
+      ) {
+        let video = content.document.querySelector("video");
+        video.id = videoID;
+
+        let script = content.document.createElement("script");
+        script.src = scriptSrc;
+        content.document.head.appendChild(script);
+      });
+
+      await testToggleHelper(browser, VIDEO_ID, false);
+    }
+  );
+});
--- a/toolkit/content/widgets/videocontrols.js
+++ b/toolkit/content/widgets/videocontrols.js
@@ -600,16 +600,17 @@ this.VideoControlsImplWidget = class {
         // _volumeControlWidth, since the volume slider implementation
         // depends on it.
         this.updateVolumeControls();
       },
 
       updatePictureInPictureToggleDisplay() {
         if (this.isAudioOnly) {
           this.pictureInPictureToggleButton.setAttribute("hidden", true);
+          this.pictureInPictureToggleExperiment.setAttribute("hidden", true);
           return;
         }
 
         if (
           this.pipToggleEnabled &&
           !this.isShowingPictureInPictureMessage &&
           VideoControlsWidget.shouldShowPictureInPictureToggle(
             this.prefs,
@@ -3150,16 +3151,17 @@ this.NoControlsDesktopImplWidget = class
 
         if (this.document.fullscreenElement) {
           this.videocontrols.setAttribute("inDOMFullscreen", true);
         }
 
         // Default the Picture-in-Picture toggle button to being hidden. We might unhide it
         // later if we determine that this video is qualified to show it.
         this.pictureInPictureToggleButton.setAttribute("hidden", true);
+        this.pictureInPictureToggleExperiment.setAttribute("hidden", true);
 
         if (this.video.readyState >= this.video.HAVE_METADATA) {
           // According to the spec[1], at the HAVE_METADATA (or later) state, we know
           // the video duration and dimensions, which means we can calculate whether or
           // not to show the Picture-in-Picture toggle now.
           //
           // [1]: https://www.w3.org/TR/html50/embedded-content-0.html#dom-media-have_metadata
           this.updatePictureInPictureToggleDisplay();