Re: [whatwg/fullscreen] Decide whether to resize synchronously in requestFullscreen() (#64)

upsuper commented on this pull request.



> +    <p>If any of the following conditions are false, then set <var>error</var> to true:
+
+    <ul>
+     <li><p>The <a>fullscreen element ready check</a> for <var>pending</var> returns true.
+     <!-- cross-process; check is only needed on pending as it is recursive already -->
+
+     <li>
+      <p><var>pending</var>'s <a>node document</a>'s <a>top-level browsing context</a>'s
+      <a>active document</a> is <var>topLevelDoc</var>.
+      <!-- cross-process -->
+
+      <p class=note><var>pending</var> may have moved to another <a>top-level browsing context</a>
+      since the request.
+    </ul>
+
+   <li><p>If <var>error</var> is true, <a>fire an event</a> named <code>fullscreenerror</code> on
    <var>pending</var>'s <a>node document</a>, reject <var>promise</var> with a
    <code>TypeError</code> exception, and terminate these steps.

This actually reminds me another issue with the old spec text: if the fullscreen request triggers resizing, but fails here, the viewport would keep fullscreen, while the content would not be in fullscreen state, which seems to be unfortunate.

Rather than simply "terminate these steps", it should probably also say "If *resize* is true, resize *topLevelDoc*’s viewport to its 'normal' dimensions" before.

(This actually caused a vulnerability in Gecko which is fixed recently. And I didn't realize this is a spec issue.)

That would still be tricky, though. If one triggers two fullscreen requests continuously, and the latter one fails the check here, then boom. But I guess it is better than leaving window in fullscreen while content is not.

> @@ -217,11 +225,26 @@ these steps:
   <p>As part of the next <a>animation frame task</a>, run these substeps:
 
   <ol>
-   <li><p>If either <var>error</var> is true or the <a>fullscreen element ready check</a> for
-   <var>pending</var> returns false, <a>fire an event</a> named <code>fullscreenerror</code> on
+
+   <li>
+    <p>If any of the following conditions are false, then set <var>error</var> to true:

I might be wrong, but I think "any" should use "is" rather than "are" here?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fullscreen/pull/64#pullrequestreview-10097951