Bug 1666775 - Make sure to hide the new Picture-in-Picture toggle on audio-only <video> elements. r=Gijs, a=RyanVM
authorMike Conley <mconley@mozilla.com>
Thu, 24 Sep 2020 18:05:39 +0000
changeset 614872 6971f3e933874c646a8f812ecaa0d84fffb7a7e1
parent 614871 64be554e3fe113b6c35735c6e471fd668c060ca6
child 614873 cfae40d60d15864511ee4c6443b02830b4efd81b
push id14068
push userryanvm@gmail.com
push dateMon, 28 Sep 2020 18:22:03 +0000
treeherdermozilla-beta@6971f3e93387 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, RyanVM
bugs1666775
milestone82.0
Bug 1666775 - Make sure to hide the new Picture-in-Picture toggle on audio-only <video> elements. r=Gijs, a=RyanVM 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_showMessage.js]
 [browser_smallVideoLayout.js]
 [browser_stripVideoStyles.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();