Skip to content
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

Do not establish close watchers in non-fully active documents #10660

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

domenic
Copy link
Member

@domenic domenic commented Sep 30, 2024

Closes #10634. See #10659 for a better followup.

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


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

@domenic domenic added topic: dialog The <dialog> element. topic: popover The popover attribute and friends topic: close watchers labels Sep 30, 2024
@lukewarlow
Copy link
Member

The easiest test here would be attach a dialog to the detached document, call .showModal(), use the sendCloseRequest() helper function and assert that the dialog doesn't actually close?

Similarly with popovers?

@keithamus
Copy link
Contributor

The easiest test here would be attach a dialog to the detached document, call .showModal(), use the sendCloseRequest() helper function and assert that the dialog doesn't actually close?

Similarly with popovers?

That'll only hold while #10659 is still open though 😂

@lukewarlow
Copy link
Member

Yeah that'll make this change superfluous though so it depends on what the desired outcome is

@domenic
Copy link
Member Author

domenic commented Oct 1, 2024

Yep, that testing strategy sounds right, although it's a bit of a silly test since it's unclear why sending an event (via sendCloseRequest()) to the main document would possibly impact the synthetic ones. And as Keith says the test would need to get changed shortly if we fix #10659. So my instinct would be to merge this without tests, as long as #10659 has some momentum...

@domenic domenic requested a review from annevk October 9, 2024 01:52
@domenic domenic merged commit 6075687 into main Oct 10, 2024
2 checks passed
@domenic domenic deleted the non-fully-active-closewatcher branch October 10, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: close watchers topic: dialog The <dialog> element. topic: popover The popover attribute and friends
Development

Successfully merging this pull request may close these issues.

It is possible to create CloseWatchers for dialog elements and popovers which aren't in fully active document
4 participants