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

Enable shadowrealm testing for dom events api #41966

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rwaldron
Copy link
Contributor

No description provided.

@annevk
Copy link
Member

annevk commented Sep 15, 2023

Let's block on this as per #41985.

rwaldron and others added 4 commits November 13, 2024 14:18
The non-ShadowRealm version of these tests rely on document.createEvent()
which is not exposed in ShadowRealms. Add separate tests for ShadowRealm
scopes that use the Event constructor instead of document.createEvent().

For the cancelBubble test, we cannot actually test the bubbling
behaviour without a DOM, but the property still must exist.

Also, make sure to expose the export on globalThis in the support script.
This adds a copy of the test in Event-propagation.html, but on a plain
EventTarget instead of a DOM node. The difference is that EventTarget
doesn't have bubbling/capture behaviour, so the expectations after
stopPropagation() and cancelBubble=true are different.

This test can be run in every global scope where EventTarget is available,
not just in ShadowRealm.
Add assertion messages, fix typo. These are here because I've added
additional variants of these tests for ShadowRealm and have made the same
changes in the new tests.
@ptomato ptomato force-pushed the rwaldron/shadowrealm-dom-events-api-tests branch from b5ac50c to cd8e252 Compare November 13, 2024 22:19
@ptomato
Copy link
Contributor

ptomato commented Nov 13, 2024

@annevk I believe this can be unblocked for the same reason as #41965. I've rebased it and added some more tests for functionality in ShadowRealms to match the DOM-dependent tests where we can't just add the "run in ShadowRealm" metadata flag.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Why add a bunch of copies of .html files instead of turning those into .any.js tests? (And ideally turning them into .any.js files would happen in a separate PR.)

@ptomato
Copy link
Contributor

ptomato commented Nov 14, 2024

@annevk Sure, I can do that. I did it this way initially, because I assumed we'd still want to keep the existing test coverage of events being created with document.createEvent() / Event.prototype.initEvent() and dispatched on DOM elements. Should I still keep those as separate .html files or do we not care much about that coverage?

@annevk
Copy link
Member

annevk commented Nov 15, 2024

Tests that cannot be generalized should be kept. They could potentially be moved to a .window.js file to avoid some boilerplate.

@ptomato
Copy link
Contributor

ptomato commented Nov 16, 2024

This is done in #49211.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants