Bug 1608358 - Fix allowfullscreen check to check for feature policy. r=baku

We can clean up this code once the feature policy pref goes away, but
meanwhile this should do the right thing.

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

dom/html/HTMLIFrameElement.cpp

@@ -154,6 +154,20 @@ nsMapRuleToAttributesFunc HTMLIFrameElement::GetAttributeMappingFunction()
   return &MapAttributesIntoRule;
 }
 
+bool HTMLIFrameElement::HasAllowFullscreenAttribute() const {
+  return GetBoolAttr(nsGkAtoms::allowfullscreen) ||
+         GetBoolAttr(nsGkAtoms::mozallowfullscreen);
+}
+
+bool HTMLIFrameElement::AllowFullscreen() const {
+  if (StaticPrefs::dom_security_featurePolicy_enabled()) {
+    // The feature policy check in Document::GetFullscreenError already accounts
+    // for the allow* attributes, so we're done.
+    return true;
+  }
+  return HasAllowFullscreenAttribute();
+}
+
 nsresult HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                                          const nsAttrValue* aValue,
                                          const nsAttrValue* aOldValue,
@@ -173,8 +187,7 @@ nsresult HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                aName == nsGkAtoms::mozallowfullscreen) {
       if (mFrameLoader) {
         if (auto* bc = mFrameLoader->GetExtantBrowsingContext()) {
-          // This could be simpler if we didn't support the prefixed
-          // attribute, then it could just use !!aValue.
+          // This can go away once we remove the featurePolicy pref.
           bc->SetFullscreenAllowedByOwner(AllowFullscreen());
         }
       }
@@ -316,7 +329,7 @@ void HTMLIFrameElement::RefreshFeaturePolicy(bool aParseAllowAttribute) {
     mFeaturePolicy->MaybeSetAllowedPolicy(u"payment"_ns);
   }
 
-  if (AllowFullscreen()) {
+  if (HasAllowFullscreenAttribute()) {
     mFeaturePolicy->MaybeSetAllowedPolicy(u"fullscreen"_ns);
   }
 

dom/html/HTMLIFrameElement.h

@@ -8,7 +8,6 @@
 #define mozilla_dom_HTMLIFrameElement_h
 
 #include "mozilla/Attributes.h"
-#include "mozilla/dom/FeaturePolicy.h"
 #include "nsGenericHTMLFrameElement.h"
 #include "nsDOMTokenList.h"
 
@@ -80,12 +79,9 @@ class HTMLIFrameElement final : public nsGenericHTMLFrameElement {
     SetHTMLBoolAttr(nsGkAtoms::allowfullscreen, aAllow, aError);
   }
 
-  // This and AllowFullscreenForBindings can be merged when we remove support
-  // for mozallowfullscreen.
-  bool AllowFullscreen() const {
-    return GetBoolAttr(nsGkAtoms::allowfullscreen) ||
-           GetBoolAttr(nsGkAtoms::mozallowfullscreen);
-  }
+  bool AllowFullscreen() const;
+
+  bool HasAllowFullscreenAttribute() const;
 
   bool AllowPaymentRequest() const {
     return GetBoolAttr(nsGkAtoms::allowpaymentrequest);

dom/html/test/file_fullscreen-denied.html

@@ -55,6 +55,10 @@ function testIFrameWithoutAllowFullscreen() {
   // we should not receive a fullscreenchange event in this document.
   var iframe = document.createElement("iframe");
   iframe.src = INNER_FILE;
+  // The iframe is same-origin so when we use feature policy otherwise we'd hit
+  // the "allowed" code-path (as intended). It is a bug that this test passes
+  // without the allow attribute.
+  iframe.allow = "fullscreen 'none'";
   setupForInnerTest("an iframe without allowfullscreen", () => {
     document.body.removeChild(iframe);
     SimpleTest.executeSoon(testFrameElement);

dom/security/featurepolicy/FeaturePolicy.cpp

@@ -311,7 +311,7 @@ void FeaturePolicy::GetAllowlistForFeature(const nsAString& aFeatureName,
 void FeaturePolicy::MaybeSetAllowedPolicy(const nsAString& aFeatureName) {
   MOZ_ASSERT(FeaturePolicyUtils::IsSupportedFeature(aFeatureName) ||
              FeaturePolicyUtils::IsExperimentalFeature(aFeatureName));
-  // Skip if feature is in experimental pharse
+  // Skip if feature is in experimental phase
   if (!StaticPrefs::dom_security_featurePolicy_experimental_enabled() &&
       FeaturePolicyUtils::IsExperimentalFeature(aFeatureName)) {
     return;

dom/security/featurepolicy/FeaturePolicyUtils.cpp

@@ -180,7 +180,7 @@ bool FeaturePolicyUtils::IsFeatureAllowed(Document* aDocument,
     return true;
   }
 
-  // Skip apply features in experimental pharse
+  // Skip apply features in experimental phase
   if (!StaticPrefs::dom_security_featurePolicy_experimental_enabled() &&
       IsExperimentalFeature(aFeatureName)) {
     return true;

testing/web-platform/meta/fullscreen/api/document-fullscreen-enabled.html.ini

@@ -0,0 +1,4 @@
+[document-fullscreen-enabled.html]
+  [Document#fullscreenEnabled]
+    expected: FAIL
+    bug: https://github.com/web-platform-tests/wpt/issues/24581

testing/web-platform/meta/html/semantics/embedded-content/the-iframe-element/iframe-allowfullscreen.html.ini

@@ -1,10 +0,0 @@
-[iframe-allowfullscreen.html]
-  [iframe-same-origin-allowfullscreen]
-    expected: FAIL
-
-  [iframe-cross-origin-allowfullscreen]
-    expected: FAIL
-
-  [iframe-noload-noallowfullscreen]
-    expected: FAIL
-

testing/web-platform/tests/fullscreen/api/document-fullscreen-enabled-cross-origin.sub.html

@@ -9,7 +9,7 @@
 <iframe src="http://{{domains[www1]}}:{{ports[http][0]}}/fullscreen/api/resources/report-fullscreen-enabled.html" allowfullscreen name="cross-origin-allow"></iframe>
 <script>
 var expectations = {
-  "same-origin-default": false,
+  "same-origin-default": true,
   "same-origin-allow": true,
   "cross-origin-default": false,
   "cross-origin-allow": true

toolkit/content/tests/widgets/test_videocontrols_iframe_fullscreen.html

@@ -13,18 +13,16 @@
 <div id="content">
 <iframe id="ifr1"></iframe>
 <iframe id="ifr2" allowfullscreen></iframe>
+<iframe id="ifr1" allow="fullscreen 'none'"></iframe>
 </div>
 
 <pre id="test">
 <script clas="testbody" type="application/javascript">
   SimpleTest.waitForExplicitFinish();
 
-  const iframe1 = document.getElementById("ifr1");
-  const iframe2 = document.getElementById("ifr2");
   const testCases = [];
 
   function checkIframeFullscreenAvailable(ifr) {
-    const available = ifr.hasAttribute("allowfullscreen");
     let video;
 
     return () => new Promise(resolve => {
@@ -35,6 +33,7 @@
       video.src = "seek_with_sound.ogg";
       video.addEventListener("loadedmetadata", resolve);
     })).then(() => new Promise(resolve => {
+      const available = video.ownerDocument.fullscreenEnabled;
       const controlBar = getElementWithinVideo(video, "controlBar");
 
       is(controlBar.getAttribute("fullscreen-unavailable") == "true", !available, "The controlbar should have an attribute marking whether fullscreen is available that corresponds to if the iframe has the allowfullscreen attribute.");
@@ -50,8 +49,8 @@
     SpecialPowers.pushPrefEnv({"set": [["media.cache_size", 40000]]}, start);
   }
 
-  testCases.push(checkIframeFullscreenAvailable(iframe1));
-  testCases.push(checkIframeFullscreenAvailable(iframe2));
+  for (let iframe of document.querySelectorAll("iframe"))
+    testCases.push(checkIframeFullscreenAvailable(iframe));
   testCases.push(SimpleTest.finish);
 
   window.addEventListener("load", load);