Re: [w3c/screen-orientation] Refactor fullscreen rejection handling to use WebIDL react pattern (PR #269)

@Copilot commented on this pull request.

## Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.





---

<sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/w3c/screen-orientation/new/gh-pages/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>

> @@ -859,19 +859,72 @@ <h2>
         <li>[=Apply orientation lock=] `null` to |topDocument|.
         </li>
       </ol>
+      <h2>
+        Rejecting descendant document orientation promises
+      </h2>
+      <p>
+        To <dfn>reject descendant document orientation promises</dfn> for a {{Document}} |document|, the [=user agent=] MUST:
+      </p>
+      <ol class="algorithm">
+        <li>If |document| is not a [=top-level browsing context=]'s [=navigable/active document=], abort these steps.</li>

The algorithm should validate whether |document| is a top-level document, but the logic appears reversed. If |document| is NOT a top-level document, the algorithm should continue processing, not abort. Consider changing this to: 'If |document| is a [=top-level browsing context=]'s [=navigable/active document=], abort these steps.' or restructure the algorithm to process descendant documents when |document| is top-level.
```suggestion
        <li>If |document| is a [=top-level browsing context=]'s [=navigable/active document=], abort these steps.</li>
```

> @@ -859,19 +859,72 @@ <h2>
         <li>[=Apply orientation lock=] `null` to |topDocument|.
         </li>
       </ol>
+      <h2>
+        Rejecting descendant document orientation promises
+      </h2>
+      <p>
+        To <dfn>reject descendant document orientation promises</dfn> for a {{Document}} |document|, the [=user agent=] MUST:
+      </p>
+      <ol class="algorithm">
+        <li>If |document| is not a [=top-level browsing context=]'s [=navigable/active document=], abort these steps.</li>
+        <li>Let |descendantDocs| be an [=ordered set=] consisting of |document|'s
+        [=Document/descendant navigables=]'s [=navigable/active documents=], if any, in tree order.</li>
+        <li>[=Set/For each=] |descendantDoc:Document| in |descendantDocs|:
+          <ol>
+            <li>If |descendantDoc| is not [=same origin=] with |document|, continue.</li>

[nitpick] Add a comment explaining why only same-origin descendant documents are processed. While the note at line 906-909 explains the same-origin requirement for the fullscreen pre-lock condition, this algorithm's same-origin restriction serves a different purpose (security/privacy) and would benefit from inline clarification.
```suggestion
            <!-- Only process same-origin descendant documents for security and privacy reasons:
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/screen-orientation/pull/269#pullrequestreview-3348349628
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/screen-orientation/pull/269/review/3348349628@github.com>