Bug 1821884 - Ensure consistent state for fullscreen/pointerlock warnings; r=Gijs

Fullscreen/PointerLock warnings are initialized with hidden="true", but
change to hidden="" after being shown and hidden again. I think this
started happening when we began using HTML elements instead of XUL as
they handle hidden attribute differently.

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

browser/base/content/browser-fullScreenAndPointerLock.js

@@ -186,7 +186,7 @@ var PointerlockFsWarning = {
     }
     if (newState != "hidden") {
       if (currentState != "hidden") {
-        this._element.setAttribute(newState, true);
+        this._element.setAttribute(newState, "");
       } else {
         // When the previous state is hidden, the display was none,
         // thus no box was constructed. We need to wait for the new
@@ -197,7 +197,7 @@ var PointerlockFsWarning = {
         requestAnimationFrame(() => {
           requestAnimationFrame(() => {
             if (this._element) {
-              this._element.setAttribute(newState, true);
+              this._element.setAttribute(newState, "");
             }
           });
         });

browser/base/content/fullscreen-and-pointerlock.inc.xhtml

@@ -3,7 +3,7 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 <html:div id="fullscreen-and-pointerlock-wrapper">
-  <html:div id="fullscreen-warning" class="pointerlockfswarning" hidden="true">
+  <html:div id="fullscreen-warning" class="pointerlockfswarning" hidden="">
     <html:div class="pointerlockfswarning-domain-text">
       <html:span class="pointerlockfswarning-domain" data-l10n-name="domain"/>
     </html:div>
@@ -20,7 +20,7 @@
     </html:button>
   </html:div>
 
-  <html:div id="pointerlock-warning" class="pointerlockfswarning" hidden="true">
+  <html:div id="pointerlock-warning" class="pointerlockfswarning" hidden="">
     <html:div class="pointerlockfswarning-domain-text">
       <html:span class="pointerlockfswarning-domain" data-l10n-name="domain"/>
     </html:div>

browser/base/content/test/fullscreen/browser_fullscreen_warning.js

@@ -3,14 +3,35 @@
 
 "use strict";
 
-add_task(async function test_fullscreen_display_none() {
+function checkWarningState(aWarningElement, aExpectedState, aMsg) {
+  ["hidden", "ontop", "onscreen"].forEach(state => {
+    is(
+      aWarningElement.hasAttribute(state),
+      state == aExpectedState,
+      `${aMsg} - check ${state} attribute.`
+    );
+  });
+}
+
+async function waitForWarningState(aWarningElement, aExpectedState) {
+  await BrowserTestUtils.waitForAttribute(aExpectedState, aWarningElement, "");
+  checkWarningState(
+    aWarningElement,
+    aExpectedState,
+    `Wait for ${aExpectedState} state`
+  );
+}
+
+add_setup(async function init() {
   await SpecialPowers.pushPrefEnv({
     set: [
       ["full-screen-api.enabled", true],
       ["full-screen-api.allow-trusted-requests-only", false],
     ],
   });
+});
 
+add_task(async function test_fullscreen_display_none() {
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
@@ -30,11 +51,13 @@ add_task(async function test_fullscreen_display_none() {
     },
     async function (browser) {
       let warning = document.getElementById("fullscreen-warning");
-      let warningShownPromise = BrowserTestUtils.waitForAttribute(
-        "onscreen",
+      checkWarningState(
         warning,
-        "true"
+        "hidden",
+        "Should not show full screen warning initially"
       );
+
+      let warningShownPromise = waitForWarningState(warning, "onscreen");
       // Enter fullscreen
       await SpecialPowers.spawn(browser, [], async () => {
         let frame = content.document.querySelector("iframe");
@@ -54,39 +77,33 @@ add_task(async function test_fullscreen_display_none() {
       );
       document.getElementById("fullscreen-exit-button").click();
       await exitFullscreenPromise;
+
+      checkWarningState(
+        warning,
+        "hidden",
+        "Should hide fullscreen warning after exiting fullscreen"
+      );
     }
   );
 });
 
 add_task(async function test_fullscreen_pointerlock_conflict() {
-  await SpecialPowers.pushPrefEnv({
-    set: [
-      ["full-screen-api.enabled", true],
-      ["full-screen-api.allow-trusted-requests-only", false],
-    ],
-  });
-
   await BrowserTestUtils.withNewTab("https://example.com", async browser => {
     let fsWarning = document.getElementById("fullscreen-warning");
     let plWarning = document.getElementById("pointerlock-warning");
 
-    is(
-      fsWarning.getAttribute("onscreen"),
-      null,
-      "Should not show full screen warning initially."
-    );
-    is(
-      plWarning.getAttribute("onscreen"),
-      null,
-      "Should not show pointer lock warning initially."
-    );
-
-    let fsWarningShownPromise = BrowserTestUtils.waitForAttribute(
-      "onscreen",
+    checkWarningState(
       fsWarning,
-      "true"
+      "hidden",
+      "Should not show full screen warning initially"
+    );
+    checkWarningState(
+      plWarning,
+      "hidden",
+      "Should not show pointer lock warning initially"
     );
 
+    let fsWarningShownPromise = waitForWarningState(fsWarning, "onscreen");
     info("Entering full screen and pointer lock.");
     await SpecialPowers.spawn(browser, [], async () => {
       await content.document.body.requestFullscreen();
@@ -94,15 +111,10 @@ add_task(async function test_fullscreen_pointerlock_conflict() {
     });
 
     await fsWarningShownPromise;
-    is(
-      fsWarning.getAttribute("onscreen"),
-      "true",
-      "Should show full screen warning."
-    );
-    is(
-      plWarning.getAttribute("onscreen"),
-      null,
-      "Should not show pointer lock warning."
+    checkWarningState(
+      plWarning,
+      "hidden",
+      "Should not show pointer lock warning"
     );
 
     info("Exiting pointerlock");
@@ -110,18 +122,19 @@ add_task(async function test_fullscreen_pointerlock_conflict() {
       await content.document.exitPointerLock();
     });
 
-    is(
-      fsWarning.getAttribute("onscreen"),
-      "true",
-      "Should still show full screen warning."
+    checkWarningState(
+      fsWarning,
+      "onscreen",
+      "Should still show full screen warning"
     );
-    is(
-      plWarning.getAttribute("onscreen"),
-      null,
-      "Should not show pointer lock warning."
+    checkWarningState(
+      plWarning,
+      "hidden",
+      "Should not show pointer lock warning"
     );
 
     // Cleanup
+    info("Exiting fullscreen");
     await document.exitFullscreen();
   });
 });

dom/tests/browser/browser_pointerlock_warning.js

@@ -15,6 +15,25 @@ const FRAME_TEST_URL =
   encodeURI(BODY_URL) +
   '"></iframe></body>';
 
+function checkWarningState(aWarningElement, aExpectedState, aMsg) {
+  ["hidden", "ontop", "onscreen"].forEach(state => {
+    is(
+      aWarningElement.hasAttribute(state),
+      state == aExpectedState,
+      `${aMsg} - check ${state} attribute.`
+    );
+  });
+}
+
+async function waitForWarningState(aWarningElement, aExpectedState) {
+  await BrowserTestUtils.waitForAttribute(aExpectedState, aWarningElement, "");
+  checkWarningState(
+    aWarningElement,
+    aExpectedState,
+    `Wait for ${aExpectedState} state`
+  );
+}
+
 // Make sure the pointerlock warning is shown and exited with the escape key
 add_task(async function show_pointerlock_warning_escape() {
   let urls = [TEST_URL, FRAME_TEST_URL];
@@ -24,11 +43,7 @@ add_task(async function show_pointerlock_warning_escape() {
     let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, url);
 
     let warning = document.getElementById("pointerlock-warning");
-    let warningShownPromise = BrowserTestUtils.waitForAttribute(
-      "onscreen",
-      warning,
-      "true"
-    );
+    let warningShownPromise = waitForWarningState(warning, "onscreen");
 
     let expectedWarningText;
 
@@ -49,11 +64,7 @@ add_task(async function show_pointerlock_warning_escape() {
 
     ok(true, "Pointerlock warning shown");
 
-    let warningHiddenPromise = BrowserTestUtils.waitForAttribute(
-      "hidden",
-      warning,
-      ""
-    );
+    let warningHiddenPromise = waitForWarningState(warning, "hidden");
 
     await BrowserTestUtils.waitForCondition(
       () => warning.innerText == expectedWarningText,