Guard showModal() and showPopover() with fully active checks by domenic · Pull Request #10705 · whatwg/html

domenic · 2024-10-16T02:48:22Z

Closes #10659. This effectively only impacts dialogs and popovers "synthetic" documents, such as those created via document.implementation.createHTMLDocument().

At least two implementers are interested (and none opposed):
- Chromium
- Gecko
Tests are written and can be reviewed and commented upon at:
- Throw exception for popovers/dialogs in non-active documents web-platform-tests/wpt#48854
Implementation bugs are filed:
- Chromium: https://issues.chromium.org/issues/373684393
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1924919
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=281550
- Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): N/A
- Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): N/A
MDN issue is filed: I've checked and MDN pages for showPopover() and showModal() do not document all the existing edge cases for exceptions, only the mainline exceptional cases. So this should not be necessary.
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

/interactive-elements.html ( diff )
/popover.html ( diff )

Closes #10659. This effectively only impacts dialogs and popovers "synthetic" documents, such as those created via document.implementation.createHTMLDocument().

maraisr · 2024-10-25T10:31:02Z

@annevk Can I take a stab at the WebKit change for this?

annevk · 2024-10-25T10:49:46Z

Please and thank you! 😊 Perhaps you're also interested in writing the tests?

maraisr · 2024-10-25T12:47:17Z

I've never done that before, but I can try. On wpt yeah?

annevk · 2024-10-25T15:58:08Z

Nice, and yes.

maraisr · 2024-10-28T02:04:10Z

@annevk I gave writing tests a go here please do point me in a direction if that is wrongly done, or what-have-you. Given that is all good, I will start with the WebKit change (and import of the tests).

See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4

maraisr · 2024-10-30T05:45:58Z

Thanks @domenic I'll close my wpt pr's then... Though must say, do not appreciate being led on like this. But all be good!

See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Web-facing change PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/jRFiIIkXv_k/m/jnPTfg8WBgAJ Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5943740 Reviewed-by: Domenic Denicola <domenic@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1375681}

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.)

…non-active documents, a=testonly Automatic update from web-platform-tests Throw exception for popovers/dialogs in non-active documents See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Web-facing change PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/jRFiIIkXv_k/m/jnPTfg8WBgAJ Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5943740 Reviewed-by: Domenic Denicola <domenic@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1375681} -- wpt-commits: 0b99ee5e8c799f9fbf7d1550ef72fec8bdb45760 wpt-pr: 48854

…non-active documents, a=testonly Automatic update from web-platform-tests Throw exception for popovers/dialogs in non-active documents See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Web-facing change PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/jRFiIIkXv_k/m/jnPTfg8WBgAJ Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5943740 Reviewed-by: Domenic Denicola <domenicchromium.org> Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Domenic Denicola <domenicchromium.org> Cr-Commit-Position: refs/heads/main{#1375681} -- wpt-commits: 0b99ee5e8c799f9fbf7d1550ef72fec8bdb45760 wpt-pr: 48854 UltraBlame original commit: ac6cc55b745eb0d675d453266bde353b7d335402

…non-active documents, a=testonly Automatic update from web-platform-tests Throw exception for popovers/dialogs in non-active documents See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Web-facing change PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/jRFiIIkXv_k/m/jnPTfg8WBgAJ Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5943740 Reviewed-by: Domenic Denicola <domenic@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1375681} -- wpt-commits: 0b99ee5e8c799f9fbf7d1550ef72fec8bdb45760 wpt-pr: 48854

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.) (cherry picked from commit 00e613c)

…non-active documents, a=testonly Automatic update from web-platform-tests Throw exception for popovers/dialogs in non-active documents See discussion at: whatwg/html#10659 and spec PR at: whatwg/html#10705 Web-facing change PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/jRFiIIkXv_k/m/jnPTfg8WBgAJ Fixed: 373684393 Change-Id: I50e400ee526775f915f006865301fff2f04016b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5943740 Reviewed-by: Domenic Denicola <domenic@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1375681} -- wpt-commits: 0b99ee5e8c799f9fbf7d1550ef72fec8bdb45760 wpt-pr: 48854

Guard showModal() and showPopover() with fully active checks

cc36e13

Closes #10659. This effectively only impacts dialogs and popovers "synthetic" documents, such as those created via document.implementation.createHTMLDocument().

domenic added normative change topic: dialog The <dialog> element topic: popover The popover attribute and friends labels Oct 16, 2024

domenic mentioned this pull request Oct 18, 2024

Should Dialog .show() close all popovers? #10708

Open

annevk approved these changes Oct 22, 2024

View reviewed changes

domenic added the needs tests Moving the issue forward requires someone to write tests label Oct 23, 2024

maraisr mentioned this pull request Oct 28, 2024

Fully active tests for showModal() and showPopover() web-platform-tests/wpt#48833

Closed

chromium-wpt-export-bot mentioned this pull request Oct 29, 2024

Throw exception for popovers/dialogs in non-active documents web-platform-tests/wpt#48854

Merged

domenic removed the needs tests Moving the issue forward requires someone to write tests label Oct 30, 2024

domenic merged commit 70f2b06 into main Oct 30, 2024

domenic deleted the no-fully-active-popover branch October 30, 2024 05:36

AtkinsSJ added a commit to AtkinsSJ/ladybird that referenced this pull request Oct 31, 2024

LibWeb/HTML: Guard showModal() with fully active check

a8b7200

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.)

AtkinsSJ mentioned this pull request Oct 31, 2024

LibWeb/HTML: Guard showModal() with fully active check LadybirdBrowser/ladybird#2077

Merged

kalenikaliaksandr pushed a commit to LadybirdBrowser/ladybird that referenced this pull request Oct 31, 2024

LibWeb/HTML: Guard showModal() with fully active check

00e613c

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.)

maraisr mentioned this pull request Oct 31, 2024

Guard dialog.showModal() and "check popover validity" with fully active checks WebKit/WebKit#35990

Merged

nico pushed a commit to nico/serenity that referenced this pull request Nov 12, 2024

LibWeb/HTML: Guard showModal() with fully active check

6fcb0eb

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.) (cherry picked from commit 00e613c)

nico pushed a commit to nico/serenity that referenced this pull request Nov 12, 2024

LibWeb/HTML: Guard showModal() with fully active check

40542cf

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.) (cherry picked from commit 00e613c)

nico pushed a commit to SerenityOS/serenity that referenced this pull request Nov 12, 2024

LibWeb/HTML: Guard showModal() with fully active check

e20a271

This corresponds to this spec change: whatwg/html#10705 (We don't implement showPopover() yet.) (cherry picked from commit 00e613c)

domenic mentioned this pull request Apr 22, 2025

Dialog requestClose causes several issues with regard to interplay of CloseWatcher #11230

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guard showModal() and showPopover() with fully active checks#10705

Guard showModal() and showPopover() with fully active checks#10705
domenic merged 1 commit intomainfrom
no-fully-active-popover

domenic commented Oct 16, 2024 •

edited by pr-preview Bot

Loading

Uh oh!

maraisr commented Oct 25, 2024

Uh oh!

annevk commented Oct 25, 2024 •

edited

Loading

Uh oh!

maraisr commented Oct 25, 2024

Uh oh!

annevk commented Oct 25, 2024

Uh oh!

maraisr commented Oct 28, 2024

Uh oh!

maraisr commented Oct 30, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Conversation

domenic commented Oct 16, 2024 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maraisr commented Oct 25, 2024

Uh oh!

annevk commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maraisr commented Oct 25, 2024

Uh oh!

annevk commented Oct 25, 2024

Uh oh!

maraisr commented Oct 28, 2024

Uh oh!

maraisr commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

domenic commented Oct 16, 2024 •

edited by pr-preview Bot

Loading

annevk commented Oct 25, 2024 •

edited

Loading

maraisr commented Oct 30, 2024 •

edited

Loading