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

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 <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/main@{#1375681}

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

…ve checks https://bugs.webkit.org/show_bug.cgi?id=281550 Reviewed by NOBODY (OOPS!). There was a WHATWG/html spec change to now guard showModal() and popover validity with fully active documents. I also ported the relevent wpt tests for this change. Spec change: <whatwg/html#10705> * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document.html: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document.html: Rebaseline. * Source/WebCore/html/HTMLDialogElement.cpp: (WebCore::HTMLDialogElement::showModal): * Source/WebCore/html/HTMLElement.cpp: (WebCore::checkPopoverValidity):

…ve checks https://bugs.webkit.org/show_bug.cgi?id=281550 Reviewed by NOBODY (OOPS!). There was a WHATWG/html spec change to now guard showModal() and popover validity with fully active documents. I also ported the relevent wpt tests for this change. Spec change: <whatwg/html#10705> * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document.html: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document.html: Rebaseline. * Source/WebCore/html/HTMLDialogElement.cpp: (WebCore::HTMLDialogElement::showModal): * Source/WebCore/html/HTMLElement.cpp: (WebCore::checkPopoverValidity):

…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 <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> 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 <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/main@{#1375681} -- wpt-commits: 0b99ee5e8c799f9fbf7d1550ef72fec8bdb45760 wpt-pr: 48854

…ve checks https://bugs.webkit.org/show_bug.cgi?id=281550 Reviewed by Darin Adler. There was a WHATWG/html spec change to now guard showModal() and popover validity with fully active documents. I also ported the relevent wpt tests for this change. Spec change: <whatwg/html#10705> * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-active-document.html: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document-expected.txt: Rebaseline. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-active-document.html: Rebaseline. * Source/WebCore/html/HTMLDialogElement.cpp: (WebCore::HTMLDialogElement::showModal): * Source/WebCore/html/HTMLElement.cpp: (WebCore::checkPopoverValidity): Canonical link: https://commits.webkit.org/286020@main

…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 <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> 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 00e613c7df7ac26ba28e11d172207c045d35a542)

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
2 checks passed

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

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 commented Oct 16, 2024 •

edited by pr-preview bot

Loading

maraisr commented Oct 25, 2024

annevk commented Oct 25, 2024 •

edited

Loading

maraisr commented Oct 25, 2024

annevk commented Oct 25, 2024

maraisr commented Oct 28, 2024

maraisr commented Oct 30, 2024 •

edited

Loading

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

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

Conversation

domenic commented Oct 16, 2024 • edited by pr-preview bot Loading

maraisr commented Oct 25, 2024

annevk commented Oct 25, 2024 • edited Loading

maraisr commented Oct 25, 2024

annevk commented Oct 25, 2024

maraisr commented Oct 28, 2024

maraisr commented Oct 30, 2024 • edited Loading

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