Bug 1651609 - Allow ESC key to exit picture-in-picture fullscreen mode. r=pip-reviewers,mhowell

This fix handles the ESC key in 2 ways:
- first when the PiP is in fullscreen mode, it would just leave the fullscreen mode
- second when the PiP is not in fullscreen mode, it would handle it as same as pressing the close button of the PiP player
- introduced a dedicated test for handling the ESC key for fullscreen and non fullscreen modes

Differential Revision: https://phabricator.services.mozilla.com/D139621

toolkit/components/pictureinpicture/content/player.js

@@ -227,15 +227,15 @@ let Player = {
       case "keydown": {
         if (event.keyCode == KeyEvent.DOM_VK_TAB) {
           this.controls.setAttribute("keying", true);
-        } else if (
-          event.keyCode == KeyEvent.DOM_VK_ESCAPE &&
-          this.controls.hasAttribute("keying")
-        ) {
-          this.controls.removeAttribute("keying");
-
-          // We preventDefault to avoid exiting fullscreen if we happen
-          // to be in it.
+        } else if (event.keyCode == KeyEvent.DOM_VK_ESCAPE) {
           event.preventDefault();
+          if (this.isFullscreen) {
+            // We handle the ESC key, in fullscreen modus as intent to leave only the fullscreen mode
+            document.exitFullscreen();
+          } else {
+            // We handle the ESC key, as an intent to leave the picture-in-picture modus
+            this.onClose();
+          }
         } else if (
           Services.prefs.getBoolPref(KEYBOARD_CONTROLS_ENABLED_PREF, false) &&
           (event.keyCode != KeyEvent.DOM_VK_SPACE || !event.target.id)
@@ -305,7 +305,7 @@ let Player = {
 
   onDblClick(event) {
     if (event.target.id == "controls") {
-      if (document.fullscreenElement == document.body) {
+      if (this.isFullscreen) {
         document.exitFullscreen();
       } else {
         document.body.requestFullscreen();
@@ -326,10 +326,7 @@ let Player = {
       }
 
       case "close": {
-        this.actor.sendAsyncMessage("PictureInPicture:Pause", {
-          reason: "pip-closed",
-        });
-        this.closePipWindow({ reason: "close-button" });
+        this.onClose();
         break;
       }
 
@@ -352,6 +349,13 @@ let Player = {
     }
   },
 
+  onClose() {
+    this.actor.sendAsyncMessage("PictureInPicture:Pause", {
+      reason: "pip-closed",
+    });
+    this.closePipWindow({ reason: "close-button" });
+  },
+
   onKeyDown(event) {
     this.actor.sendAsyncMessage("PictureInPicture:KeyDown", {
       altKey: event.altKey,
@@ -625,6 +629,15 @@ let Player = {
     document.l10n.setAttributes(audioButton, strId);
   },
 
+  /**
+   * GET isFullscreen returns true if the video is running in fullscreen mode
+   *
+   * @returns {boolean}
+   */
+  get isFullscreen() {
+    return document.fullscreenElement == document.body;
+  },
+
   /**
    * Used for recording telemetry in Picture-in-Picture.
    *

toolkit/components/pictureinpicture/tests/browser.ini

@@ -57,6 +57,7 @@ skip-if =
 skip-if = (os == "mac" && debug) || os == "linux" #Bug 1566173, Bug 1664667
 [browser_keyboardShortcut.js]
 [browser_keyboardShortcutClosePIP.js]
+[browser_keyboardClosePIPwithESC.js]
 [browser_keyboardShortcutWithNanDuration.js]
 support-files =
   test-page-with-nan-video-duration.html

toolkit/components/pictureinpicture/tests/browser_keyboardClosePIPwithESC.js

@@ -0,0 +1,71 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * tests that the ESC key would stop the player and close the PiP player floating window
+ */
+add_task(async () => {
+  for (let videoID of ["with-controls", "no-controls"]) {
+    info(`Testing ${videoID} case.`);
+
+    let playVideo = () => {
+      return SpecialPowers.spawn(browser, [videoID], async videoID => {
+        return content.document.getElementById(videoID).play();
+      });
+    };
+
+    let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE);
+    let browser = tab.linkedBrowser;
+
+    await playVideo();
+
+    let pipWin = await triggerPictureInPicture(browser, videoID);
+    ok(pipWin, "The Picture-in-Picture window is not there.");
+    ok(
+      !(await isVideoPaused(browser, videoID)),
+      "The video is paused, but should not."
+    );
+    ok(
+      !pipWin.document.fullscreenElement,
+      "PiP should not yet be in fullscreen."
+    );
+
+    let controls = pipWin.document.getElementById("controls");
+    await promiseFullscreenEntered(pipWin, async () => {
+      EventUtils.sendMouseEvent({ type: "dblclick" }, controls, pipWin);
+    });
+
+    ok(
+      pipWin.document.fullscreenElement == pipWin.document.body,
+      "Double-click should have caused to enter fullscreen."
+    );
+
+    await promiseFullscreenExited(pipWin, async () => {
+      EventUtils.synthesizeKey("KEY_Escape", {}, pipWin);
+    });
+
+    ok(
+      !pipWin.document.fullscreenElement,
+      "ESC should have caused to leave fullscreen."
+    );
+    ok(
+      !(await isVideoPaused(browser, videoID)),
+      "The video is paused, but should not."
+    );
+
+    // Try to close the PiP window via the ESC button, since now it is not in fullscreen anymore.
+    EventUtils.synthesizeKey("KEY_Escape", {}, pipWin);
+
+    // then the PiP should have been closed
+    ok(pipWin.closed, "Picture-in-Picture window is not closed, but should.");
+    // and the video should not be playing anymore
+    ok(
+      await isVideoPaused(browser, videoID),
+      "The video is not paused, but should."
+    );
+
+    await BrowserTestUtils.removeTab(tab);
+  }
+});