-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 URLPattern tests in ShadowRealm #49323
base: master
Are you sure you want to change the base?
Conversation
There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you! |
5e3217a
to
92bf5d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked to see if these tests pass today (in any engines which implement this today, which might still be Chromium)? Can you file implementation bugs against any browser where they don't pass?
When I try patching this in, I get this failure inside the WPT manifest code:
AssertionError: duplicate URL 'urlpattern/urlpattern-compare.tentative.https.any.shadowrealm-in-audioworklet.html
Possibly shadowrealm
causes the non-https
one to also generate such a file, and therefore it should only be listed in the .https
one? (I'm not very familiar with how the WPT shadowrealm environment is generated.)
@@ -1,2 +1,2 @@ | |||
// META: global=window,worker | |||
// META: global=window,worker,shadowrealm-in-window,shadowrealm-in-shadowrealm,shadowrealm-in-dedicatedworker,shadowrealm-in-sharedworker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do most of these specify shadowrealm
but this one specifies this list instead? What is the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these are the subset of shadowrealm
? Maybe it's better to set shadowrealm
.
wpt/tools/manifest/sourcefile.py
Lines 100 to 106 in 2b8ebc2
"shadowrealm": {"longhand": { | |
"shadowrealm-in-window", | |
"shadowrealm-in-shadowrealm", | |
"shadowrealm-in-dedicatedworker", | |
"shadowrealm-in-sharedworker", | |
"shadowrealm-in-serviceworker", | |
"shadowrealm-in-audioworklet", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left out shadowrealm-in-serviceworker
and shadowrealm-in-audioworklet
because service workers and audio worklets can only be created in an HTTPS context, so they'd be generated with .https.
anyway, and then be duplicates of the ones generated from shadowrealm.https.any.js
.
Requires using fetch_json to download the test data.
Unfortunately, the intersection of engines which implement URLPattern and engines which implement ShadowRealm currently seems to be nil 😄 I'll tag @lukewarlow in here who was working on an experimental Chromium implementation of ShadowRealm at one point, to see if it had progressed far enough to check whether these tests pass.
Sure, will do as part of the checklist in whatwg/urlpattern#236.
I think I missed that the |
92bf5d0
to
cb492ea
Compare
URLPattern should be exposed in chrome if you enable the right flag. But chrome doesn't support importValue or dynamic imports in shadow realms so can't run WPTs unfortunately. It also doesn't yet support shadow realm (or other Exposed=* APIs) in worklets |
Requires using fetch_json to download the test data.
See whatwg/urlpattern#236 for spec decision.