Bug 1591253 - Maintain PiP player position when source video is resized r=mconley
☠☠ backed out by 96b4cb315770 ☠ ☠
authorMark Striemer <mstriemer@mozilla.com>
Thu, 14 Nov 2019 00:32:05 +0000
changeset 502000 f7cc2f6d9640820f5278bbc7787ccae542e06e69
parent 501999 bc637c20c7d4c06835d3269e9a3558feacb6c28e
child 502001 d395b70f3ff8af258f4ccb2e78cef02c27cf9ff4
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1591253
milestone72.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 1591253 - Maintain PiP player position when source video is resized r=mconley Differential Revision: https://phabricator.services.mozilla.com/D52064
toolkit/components/pictureinpicture/PictureInPicture.jsm
toolkit/components/pictureinpicture/tests/browser_resizeVideo.js
--- a/toolkit/components/pictureinpicture/PictureInPicture.jsm
+++ b/toolkit/components/pictureinpicture/PictureInPicture.jsm
@@ -417,16 +417,53 @@ var PictureInPicture = {
         // the original aspect ratio. Since aspect ratio is width over height,
         // this means we need to _multiply_ the MAX_HEIGHT by the aspect ratio
         // to calculate the appropriate width.
         height = MAX_HEIGHT;
         width = Math.round(MAX_HEIGHT * aspectRatio);
       }
     }
 
+    // Figure out where to position the window on screen. If we have a player
+    // window this will account for any change in video size. Otherwise the
+    // video will be positioned in the bottom right.
+
+    if (isPlayerWindow) {
+      // We might need to move the window to keep its positioning in a similar
+      // part of the screen.
+      //
+      // Find the distance from each edge of the screen of the old video, we'll
+      // keep the closest edge in the same spot.
+      let prevWidth = windowOrPlayer.innerWidth;
+      let prevHeight = windowOrPlayer.innerHeight;
+      let distanceLeft = windowOrPlayer.screenX;
+      let distanceRight =
+        screenWidth.value - windowOrPlayer.screenX - prevWidth;
+      let distanceTop = windowOrPlayer.screenY;
+      let distanceBottom =
+        screenHeight.value - windowOrPlayer.screenY - prevHeight;
+
+      let left = windowOrPlayer.screenX;
+      let top = windowOrPlayer.screenY;
+
+      if (distanceRight < distanceLeft) {
+        // Closer to the right edge than the left. Move the window right by
+        // the difference in the video widths.
+        left += prevWidth - width;
+      }
+
+      if (distanceBottom < distanceTop) {
+        // Closer to the bottom edge than the top. Move the window down by
+        // the difference in the video heights.
+        top += prevHeight - height;
+      }
+
+      return { top, left, width, height };
+    }
+
     // Now that we have the dimensions of the video, we need to figure out how
     // to position it in the bottom right corner. Since we know the width of the
     // available rect, we need to subtract the dimensions of the window we're
     // opening to get the top left coordinates that openWindow expects.
     //
     // In event that the user has multiple displays connected, we have to
     // calculate the top-left coordinate of the new window in absolute
     // coordinates that span the entire display space, since this is what the
@@ -447,18 +484,19 @@ var PictureInPicture = {
 
   resizePictureInPictureWindow(videoData) {
     let win = this.getWeakPipPlayer();
 
     if (!win) {
       return;
     }
 
-    let { width, height } = this.fitToScreen(win, videoData);
+    let { top, left, width, height } = this.fitToScreen(win, videoData);
     win.resizeTo(width, height);
+    win.moveTo(left, top);
   },
 
   openToggleContextMenu(window, data) {
     let document = window.document;
     let popup = document.getElementById("pictureInPictureToggleContextMenu");
 
     // We synthesize a new MouseEvent to propagate the inputSource to the
     // subsequently triggered popupshowing event.
--- a/toolkit/components/pictureinpicture/tests/browser_resizeVideo.js
+++ b/toolkit/components/pictureinpicture/tests/browser_resizeVideo.js
@@ -1,110 +1,249 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 /**
+ * Run the resize test on a player window.
+ *
+ * @param browser (xul:browser)
+ *   The browser that has the source video.
+ *
+ * @param videoID (string)
+ *   The id of the video in the browser to test.
+ *
+ * @param pipWin (player window)
+ *   A player window to run the tests on.
+ *
+ * @param opts (object)
+ *   The options for the test.
+ *
+ *   pinX (boolean):
+ *     If true, the video's X position shouldn't change when resized.
+ *
+ *   pinY (boolean):
+ *     If true, the video's Y position shouldn't change when resized.
+ */
+async function testVideo(browser, videoID, pipWin, { pinX, pinY } = {}) {
+  async function switchVideoSource(src) {
+    let videoResized = BrowserTestUtils.waitForEvent(pipWin, "resize");
+    await ContentTask.spawn(
+      browser,
+      { src, videoID },
+      async ({ src, videoID }) => {
+        let doc = content.document;
+        let video = doc.getElementById(videoID);
+        video.src = src;
+      }
+    );
+    await videoResized;
+  }
+
+  /**
+   * Check the new screen position against the previous one. When
+   * pinX or pinY is true then the top left corner is checked in that
+   * dimension. Otherwise, the bottom right corner is checked.
+   *
+   * The video position is determined by the screen edge it's closest
+   * to, so in the default bottom right its bottom right corner should
+   * match the previous video's bottom right corner. For the top left,
+   * the top left corners should match.
+   */
+  function checkPosition(
+    previousScreenX,
+    previousScreenY,
+    previousWidth,
+    previousHeight,
+    newScreenX,
+    newScreenY,
+    newWidth,
+    newHeight
+  ) {
+    if (pinX) {
+      Assert.equal(
+        previousScreenX,
+        newScreenX,
+        "New video is still in the same X position"
+      );
+    } else {
+      Assert.equal(
+        previousScreenX + previousWidth,
+        newScreenX + newWidth,
+        "New video ends at the same screen X position"
+      );
+    }
+    if (pinY) {
+      Assert.equal(
+        previousScreenY,
+        newScreenY,
+        "New video is still in the same Y position"
+      );
+    } else {
+      Assert.equal(
+        previousScreenY + previousHeight,
+        newScreenY + newHeight,
+        "New video ends at the same screen Y position"
+      );
+    }
+  }
+
+  Assert.ok(pipWin, "Got PiP window.");
+
+  let initialWidth = pipWin.innerWidth;
+  let initialHeight = pipWin.innerHeight;
+  let initialAspectRatio = initialWidth / initialHeight;
+  Assert.equal(
+    Math.floor(initialAspectRatio * 100),
+    177, // 16 / 9 = 1.777777777
+    "Original aspect ratio is 16:9"
+  );
+
+  // Store the window position for later.
+  let initialScreenX = pipWin.mozInnerScreenX;
+  let initialScreenY = pipWin.mozInnerScreenY;
+
+  await switchVideoSource("test-video-cropped.mp4");
+
+  let resizedWidth = pipWin.innerWidth;
+  let resizedHeight = pipWin.innerHeight;
+  let resizedAspectRatio = resizedWidth / resizedHeight;
+  Assert.equal(
+    Math.floor(resizedAspectRatio * 100),
+    133, // 4 / 3 = 1.333333333
+    "Resized aspect ratio is 4:3"
+  );
+  Assert.equal(initialWidth, resizedWidth, "Resized video has the same width");
+  Assert.greater(resizedHeight, initialHeight, "Resized video grew vertically");
+
+  let resizedScreenX = pipWin.mozInnerScreenX;
+  let resizedScreenY = pipWin.mozInnerScreenY;
+  checkPosition(
+    initialScreenX,
+    initialScreenY,
+    initialWidth,
+    initialHeight,
+    resizedScreenX,
+    resizedScreenY,
+    resizedWidth,
+    resizedHeight
+  );
+
+  await switchVideoSource("test-video-vertical.mp4");
+
+  let verticalWidth = pipWin.innerWidth;
+  let verticalHeight = pipWin.innerHeight;
+  let verticalAspectRatio = verticalWidth / verticalHeight;
+  Assert.equal(
+    Math.floor(verticalAspectRatio * 100),
+    50, // 1 / 2 = 0.5
+    "Vertical aspect ratio is 1:2"
+  );
+  Assert.less(verticalWidth, resizedWidth, "Vertical video width shrunk");
+  Assert.equal(
+    resizedWidth,
+    verticalHeight,
+    "Vertical video height matches previous width"
+  );
+
+  let verticalScreenX = pipWin.mozInnerScreenX;
+  let verticalScreenY = pipWin.mozInnerScreenY;
+  checkPosition(
+    resizedScreenX,
+    resizedScreenY,
+    resizedWidth,
+    resizedHeight,
+    verticalScreenX,
+    verticalScreenY,
+    verticalWidth,
+    verticalHeight
+  );
+
+  await switchVideoSource("test-video.mp4");
+
+  let restoredWidth = pipWin.innerWidth;
+  let restoredHeight = pipWin.innerHeight;
+  let restoredAspectRatio = restoredWidth / restoredHeight;
+  Assert.equal(
+    Math.floor(restoredAspectRatio * 100),
+    177,
+    "Restored aspect ratio is still 16:9"
+  );
+  Assert.equal(
+    initialWidth,
+    pipWin.innerWidth,
+    "Restored video has its original width"
+  );
+  Assert.equal(
+    initialHeight,
+    pipWin.innerHeight,
+    "Restored video has its original height"
+  );
+
+  let restoredScreenX = pipWin.mozInnerScreenX;
+  let restoredScreenY = pipWin.mozInnerScreenY;
+  checkPosition(
+    initialScreenX,
+    initialScreenY,
+    initialWidth,
+    initialHeight,
+    restoredScreenX,
+    restoredScreenY,
+    restoredWidth,
+    restoredHeight
+  );
+}
+
+/**
  * Tests that if a <video> element is resized the Picture-in-Picture window
  * will be resized to match the new dimensions.
  */
 add_task(async () => {
   for (let videoID of ["with-controls", "no-controls"]) {
     info(`Testing ${videoID} case.`);
 
     await BrowserTestUtils.withNewTab(
       {
         url: TEST_PAGE,
         gBrowser,
       },
       async browser => {
         let pipWin = await triggerPictureInPicture(browser, videoID);
 
-        async function switchVideoSource(src) {
-          let videoResized = BrowserTestUtils.waitForEvent(pipWin, "resize");
-          await ContentTask.spawn(
-            browser,
-            { src, videoID },
-            async ({ src, videoID }) => {
-              let doc = content.document;
-              let video = doc.getElementById(videoID);
-              video.src = src;
-            }
-          );
-          await videoResized;
-        }
-
-        Assert.ok(pipWin, "Got PiP window.");
-
-        let initialWidth = pipWin.innerWidth;
-        let initialHeight = pipWin.innerHeight;
-        let initialAspectRatio = initialWidth / initialHeight;
-        Assert.equal(
-          Math.floor(initialAspectRatio * 100),
-          177, // 16 / 9 = 1.777777777
-          "Original aspect ratio is 16:9"
-        );
-
-        await switchVideoSource("test-video-cropped.mp4");
+        await testVideo(browser, videoID, pipWin);
 
-        let resizedWidth = pipWin.innerWidth;
-        let resizedHeight = pipWin.innerHeight;
-        let resizedAspectRatio = resizedWidth / resizedHeight;
-        Assert.equal(
-          Math.floor(resizedAspectRatio * 100),
-          133, // 4 / 3 = 1.333333333
-          "Resized aspect ratio is 4:3"
-        );
-        Assert.equal(
-          initialWidth,
-          resizedWidth,
-          "Resized video has the same width"
-        );
-        Assert.greater(
-          resizedHeight,
-          initialHeight,
-          "Resized video grew vertically"
-        );
-
-        await switchVideoSource("test-video-vertical.mp4");
+        pipWin.moveTo(0, 0);
 
-        let verticalAspectRatio = pipWin.innerWidth / pipWin.innerHeight;
-        Assert.equal(
-          Math.floor(verticalAspectRatio * 100),
-          50, // 1 / 2 = 0.5
-          "Vertical aspect ratio is 1:2"
-        );
-        Assert.less(
-          pipWin.innerWidth,
-          resizedWidth,
-          "Vertical video width shrunk"
-        );
-        Assert.equal(
-          resizedWidth,
-          pipWin.innerHeight,
-          "Vertical video height matches previous width"
-        );
-
-        await switchVideoSource("test-video.mp4");
-
-        let restoredAspectRatio = pipWin.innerWidth / pipWin.innerHeight;
-        Assert.equal(
-          Math.floor(restoredAspectRatio * 100),
-          177,
-          "Restored aspect ratio is still 16:9"
-        );
-        Assert.equal(
-          initialWidth,
-          pipWin.innerWidth,
-          "Restored video has its original width"
-        );
-        Assert.equal(
-          initialHeight,
-          pipWin.innerHeight,
-          "Restored video has its original height"
-        );
+        await testVideo(browser, videoID, pipWin, { pinX: true, pinY: true });
 
         await BrowserTestUtils.closeWindow(pipWin);
       }
     );
   }
 });
+
+/**
+ * Tests that the RTL video starts on the left and is pinned in the X dimension.
+ */
+add_task(async () => {
+  await SpecialPowers.pushPrefEnv({ set: [["intl.l10n.pseudo", "bidi"]] });
+
+  for (let videoID of ["with-controls", "no-controls"]) {
+    info(`Testing ${videoID} case.`);
+
+    await BrowserTestUtils.withNewTab(
+      {
+        url: TEST_PAGE,
+        gBrowser,
+      },
+      async browser => {
+        let pipWin = await triggerPictureInPicture(browser, videoID);
+
+        await testVideo(browser, videoID, pipWin, { pinX: true });
+
+        await BrowserTestUtils.closeWindow(pipWin);
+      }
+    );
+  }
+
+  await SpecialPowers.popPrefEnv();
+});