-
Notifications
You must be signed in to change notification settings - Fork 48
Add pending fullscreen request flag and promise to document state #251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add pending fullscreen request flag and promise to document state #251
Conversation
This commit adds exported definitions for both 'pending fullscreen request flag' and 'pending fullscreen request promise' to document state, enabling other specifications to use modern WebIDL patterns. Changes: - Add 'pending fullscreen request flag' to document state (exported) - Add 'pending fullscreen request promise' to document state (exported) - Set flag and store promise when requestFullscreen() starts processing - Clear flag and promise when request resolves (success) or rejects (error) - Use consistent naming conventions without interface binding The flag enables detecting pending fullscreen requests, while the promise enables other specifications to use WebIDL 'react' patterns to respond to fullscreen request rejections instead of relying on flag state changes. This enables other specifications (like Screen Orientation) to detect when a document has a pending fullscreen request and react to promise rejections, allowing modern promise-based integration for web compatibility. Related to w3c/screen-orientation#254 and w3c/screen-orientation#255
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Fullscreen API specification by adding exported document state for tracking pending fullscreen requests. The changes enable other specifications (like Screen Orientation) to detect and react to fullscreen requests using modern WebIDL promise patterns rather than relying on state flag changes.
Key Changes:
- Introduces two new exported document state definitions: a flag to indicate pending requests and a promise to enable promise-based reactions
- Updates the requestFullscreen() algorithm to set/clear the flag and promise at appropriate points in the request lifecycle
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
annevk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vaguely recall promises not being a great building block for specifications, but I can't find the specific issue for this.
| <p>All <a for=/>documents</a> have an associated <dfn>list of pending fullscreen events</dfn>, which | ||
| is an <a>ordered set</a> of (<a>string</a>, <a>element</a>) <a>tuples</a>. It is initially empty. | ||
|
|
||
| <p>All <a for=/>documents</a> have an associated <dfn export>pending fullscreen request flag</dfn>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename this to "has pending fullscreen request" and make it a boolean. Should also maybe use for=Document?
| <li><p>If <var>error</var> is false, then <a>consume user activation</a> given | ||
| <var>pendingDoc</var>'s <a>relevant global object</a>. | ||
|
|
||
| <li><p>If <var>error</var> is false, set <var>pendingDoc</var>'s <a>pending fullscreen request</a> flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then set
| <p>If <var>error</var> is true: | ||
|
|
||
| <ol> | ||
| <li><p>Unset <var>pendingDoc</var>'s <a>pending fullscreen request</a> flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set ... to false.
| Unless stated otherwise it is unset. When set, it indicates that the document has an outstanding | ||
| fullscreen request that has not yet resolved or been rejected. | ||
|
|
||
| <p>All <a for=/>documents</a> have an associated <dfn export>pending fullscreen request promise</dfn>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer it if these follow the existing pattern:
All documents have an associated X, which is a Y. It is initially Z.
Motivated by w3c/screen-orientation#254 and w3c/screen-orientation#255 and w3c/screen-orientation#269
This commit adds exported definitions for both 'pending fullscreen request flag' and 'pending fullscreen request promise' to document state, enabling other specifications to use modern WebIDL patterns.
Changes:
The flag enables detecting pending fullscreen requests, while the promise enables other specifications to use WebIDL 'react' patterns to respond to fullscreen request rejections instead of relying on flag state changes.
This enables other specifications (like Screen Orientation) to detect when a document has a pending fullscreen request and react to promise rejections, allowing modern promise-based integration for web compatibility.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff