Skip to content

Conversation

@keithamus
Copy link
Member

@keithamus keithamus commented Jan 26, 2024

bd7649e

Implement toggleevents for dialog open/close
https://bugs.webkit.org/show_bug.cgi?id=268160

Reviewed by Tim Nguyen.

This adds the `beforetoggle` cancelable event to `show()`/`showModal()`
calls. If `beforetoggle` isn't cancelled, it will show the dialog and
dispatch a `toggle` event.

Likewise, the close method adds a NON cancelable `beforetoggle` and
`toggle` event.

These events can help developers when prepping a dialog to be shown or
hidden, such as doing page calculations or kicking off animations.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/toggle-events.tentative-expected.txt:
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::show):
(WebCore::HTMLDialogElement::showModal):
(WebCore::HTMLDialogElement::close):
(WebCore::HTMLDialogElement::queueDialogToggleEventTask):
* Source/WebCore/html/HTMLDialogElement.h:

Canonical link: https://commits.webkit.org/289695@main

9c17339

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@keithamus keithamus requested a review from nt1m January 26, 2024 23:42
@webkit-early-warning-system

This comment was marked as outdated.

@nt1m
Copy link
Member

nt1m commented Jan 26, 2024

Is this is in the spec somewhere?

@Ahmad-S792 Ahmad-S792 added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Jan 26, 2024
@keithamus keithamus force-pushed the dialog-toggle-events branch from ef08e8f to 706583c Compare January 26, 2024 23:51
@webkit-early-warning-system

This comment was marked as outdated.

@keithamus
Copy link
Member Author

Is this is in the spec somewhere?

Not yet but we discussed it as TPAC and it was deemed a reasonable addition. Tentative spec PR here: whatwg/html#10091

Comment on lines 147 to 181
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the close event should be emitted before toggle, to get consistent event ordering (otherwise toggle might fire first when no coalescing is happening, but after when it isn't)

@webkit-early-warning-system

This comment was marked as outdated.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 5, 2024
@woody-li
Copy link

The spec PR has been merged.

Copy link
Member

@nt1m nt1m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure tests are passing before landing

@keithamus keithamus removed the merging-blocked Applied to prevent a change from being merged label Jan 18, 2025
@keithamus keithamus force-pushed the dialog-toggle-events branch from 9fcacba to fa02c68 Compare January 18, 2025 08:55
@webkit-early-warning-system

This comment was marked as outdated.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 18, 2025
@lukewarlow lukewarlow force-pushed the dialog-toggle-events branch from fa02c68 to 9c17339 Compare February 1, 2025 22:42
@lukewarlow lukewarlow removed the merging-blocked Applied to prevent a change from being merged label Feb 1, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 2, 2025
@lukewarlow lukewarlow removed the merging-blocked Applied to prevent a change from being merged label Feb 2, 2025
@keithamus keithamus added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Feb 2, 2025
@webkit-ews-buildbot webkit-ews-buildbot added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Feb 2, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safe-Merge-Queue: Build #47598.

https://bugs.webkit.org/show_bug.cgi?id=268160

Reviewed by Tim Nguyen.

This adds the `beforetoggle` cancelable event to `show()`/`showModal()`
calls. If `beforetoggle` isn't cancelled, it will show the dialog and
dispatch a `toggle` event.

Likewise, the close method adds a NON cancelable `beforetoggle` and
`toggle` event.

These events can help developers when prepping a dialog to be shown or
hidden, such as doing page calculations or kicking off animations.

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/toggle-events.tentative-expected.txt:
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::show):
(WebCore::HTMLDialogElement::showModal):
(WebCore::HTMLDialogElement::close):
(WebCore::HTMLDialogElement::queueDialogToggleEventTask):
* Source/WebCore/html/HTMLDialogElement.h:

Canonical link: https://commits.webkit.org/289695@main
@webkit-commit-queue
Copy link
Collaborator

Committed 289695@main (bd7649e): https://commits.webkit.org/289695@main

Reviewed commits have been landed. Closing PR #23332 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit bd7649e into WebKit:main Feb 2, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 2, 2025
@keithamus keithamus deleted the dialog-toggle-events branch February 16, 2025 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants