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.

Sign up for GitHub

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

Jump to bottom

Remove tests related to whatwg/html#9571 from Interop 2023 #406

Closed
nt1m opened this issue Sep 7, 2023 · 10 comments · Fixed by web-platform-tests/wpt-metadata#4780
Closed

Remove tests related to whatwg/html#9571 from Interop 2023 #406

nt1m opened this issue Sep 7, 2023 · 10 comments · Fixed by web-platform-tests/wpt-metadata#4780
Labels
focus area: Modules test-change-proposal Proposal to add or remove tests for an interop area

Comments

@nt1m
Copy link
Member

nt1m commented Sep 7, 2023 •
edited
Loading

Test List

  • workers/modules/dedicated-worker-import-csp.html
  • workers/modules/shared-worker-import-csp.html
  • workers/modules/dedicated-worker-import-data-url-cross-origin.html
  • workers/modules/shared-worker-import-data-url-cross-origin.html
    • static import script from data: URL should be allowed. (split this subtest out)
  • workers/modules/dedicated-worker-import-data-url.any.html
  • workers/modules/dedicated-worker-import-data-url.any.worker.html
  • workers/modules/shared-worker-import-data-url.window.html
    • [Static import (redirect).] (split this subtest out)

Rationale

Exact behavior is pending on spec discussion in whatwg/html#9571
@nt1m nt1m added test-change-proposal Proposal to add or remove tests for an interop area focus area: Modules labels Sep 7, 2023
@nt1m
Copy link
Member Author

nt1m commented Sep 7, 2023

@nairnandu / @jgraham Can you find someone to review for Blink / Gecko?

@nairnandu
Copy link
Contributor

nairnandu commented Sep 7, 2023 •
edited
Loading

@nyaxt/@hayatoito could you review this test change proposal for Blink?

@nt1m
Copy link
Member Author

nt1m commented Sep 8, 2023

@youennf Can you check if the list of tests affected by whatwg/html#9571 is exhaustive?

cc @allstarschh

@allstarschh
Copy link

Hi, thanks for adding me in.

I think there are some other tests also affected by whatwg/html#9571

  • /workers/modules/dedicated-worker-import-data-url-cross-origin.html

  • /workers/modules/shared-worker-import-data-url-cross-origin.html

    • [static import script from data: URL should be allowed.]

  • /workers/modules/dedicated-worker-import-data-url.any.html

  • /workers/modules/dedicated-worker-import-data-url.any.worker.html

  • /workers/modules/shared-worker-import-data-url.window.html

    • [Static import (redirect).]

Please see the summary in
https://phabricator.services.mozilla.com/D187900
https://phabricator.services.mozilla.com/D187901

cc @smaug----

@youennf
Copy link

youennf commented Sep 11, 2023

Agreed with the additional tests plus shared-worker-import-csp.html as well.

@nt1m nt1m changed the title Remove workers/modules/dedicated-worker-import-csp.html from Interop 2023 Remove tests related to whatwg/html#9571 from Interop 2023 Sep 11, 2023
@hayatoito
Copy link
Member

hayatoito commented Sep 12, 2023 •
edited
Loading

I am not sure whether whatwg/html#9571 is the right direction or not.

That being said, it's likely that the following tests could also be impacted by the issue, as they all utilize imports within workers:

workers/modules/shared-worker-parse-error-failure.html
workers/modules/shared-worker-import-failure.html
workers/modules/dedicated-worker-import-failure.html

Could you remove these tests as well?

These are tracked on https://crbug.com/1446246 on Blink.

@allstarschh
Copy link

allstarschh commented Sep 12, 2023 •
edited
Loading

Hi, @hayatoito
Those tests are correct, and not related to whatwg/html#9571

That being said, it's likely that the following tests could also be impacted by the issue, as they all utilize imports within workers:

workers/modules/shared-worker-parse-error-failure.html

Please see domenic's comment from whatwg/html#2289 (comment)

workers/modules/shared-worker-import-failure.html
workers/modules/dedicated-worker-import-failure.html

See web-platform-tests/wpt#41745
and also web-platform-tests/wpt#41745 (comment)
for the explanation of the current spec.
Chrome and Webkit still use the legacy behavior, which is not correct anymore.

@hayatoito
Copy link
Member

Thanks! I understand these tests are not related to the whatwg/html#9571.

I haven't had the time to fully understand the current standard yet,
so you can ignore my previous comment and proceed.

@foolip
Copy link
Member

foolip commented Sep 15, 2023

@nt1m would you mind preparing a PR against wpt-metadata to make it very easy to see which tests should be removed, given the many comments on this issue?

allstarschh added a commit to allstarschh/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh
Remove [dedicated|shared]-worker-import-csp.html from interop-2023-mo…
f67c41e 
…dules

Fixing web-platform-tests/interop#406

The test [dedicated|shared]-worker-import-csp.html will inject CSP
headers on the top-level document for static import.

https://github.com/web-platform-tests/wpt/blob/256877037fa53c4c90e48e7171a69ea0cef0d3be/workers/modules/dedicated-worker-import-csp.html#L21

And the top-level module worker script will 'static import' its
descendants.
https://github.com/web-platform-tests/wpt/blob/master/workers/modules/resources/static-import-remote-origin-script-worker.sub.js

But due to the spec issue whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-csp.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-csp.html?label=experimental&label=master&aligned

So removing these two tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these two files from
interop-2023-modules completely)
allstarschh added a commit to allstarschh/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh
Remove [shared|dedicated]-worker-import-data-url-cross-origin.html fr…
7f7a30b 
…om interop-2023-modules

Fixing web-platform-tests/interop#406

In the [static import script from data: URL should be allowed.] case,
The worker will load the data URI, which imports "/resources/static-import-script-block-cross-origin.js".

https://github.com/web-platform-tests/wpt/blob/master/workers/modules/dedicated-worker-import-data-url-cross-origin.html#L13

And static-import-script-block-cross-origin.js will static import 'export-block-cross-origin.js'

https://github.com/web-platform-tests/wpt/blob/master/workers/modules/resources/static-import-script-block-cross-origin.js

The script "export-block-cross-origin.js" is same-origin with the
document, but not same-origin with the worker script which is loaded by
data: URI.

Due to the spec issue, whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url-cross-origin.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-data-url-cross-origin.html?label=experimental&label=master&aligned

So removing these two tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these two files from
interop-2023-modules completely)
allstarschh added a commit to allstarschh/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh
Remove dedicated-worker-import-data-url.any(.worker).html and shared-…
643fc70 
…worker-import-data-url.window.html from interop-2023-modules

Fixing web-platform-tests/interop#406

In this test, The worker will load a data URI, which import another test
script statically, and the imported script will import 'export-on-load-script.js',
which will be served with the CORS header 'Access-Control-Allow-Origin: *' in
/workers/modules/resources/export-on-load-script.js.headers

In the test case [Static import (redirect).], 'export-on-load-script.js' will be
redirected, and the redirected script doesn't have the CORS header.

https://github.com/web-platform-tests/wpt/blob/2cd449b9df6f92862abac0143ac6fe674fd5278e/workers/modules/resources/import-test-cases.js#L13
https://github.com/web-platform-tests/wpt/blob/2cd449b9df6f92862abac0143ac6fe674fd5278e/workers/modules/resources/static-import-redirect-worker.js#L3

The script "export-on-load-script.js" is same-origin with the
document, but not same-origin with the worker script which is loaded by
data: URI.

Due to the spec issue, whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url.any.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url.any.worker.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-data-url.window.html?label=experimental&label=master&aligned

So removing these three tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these three files from
interop-2023-modules completely)
@allstarschh
Copy link

PR is sent in web-platform-tests/wpt-metadata#4780

@nt1m nt1m mentioned this issue Sep 21, 2023
Merged
nt1m pushed a commit to web-platform-tests/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh @nt1m
Remove [dedicated|shared]-worker-import-csp.html from interop-2023-mo…
d987035 
…dules

Fixing web-platform-tests/interop#406

The test [dedicated|shared]-worker-import-csp.html will inject CSP
headers on the top-level document for static import.

https://github.com/web-platform-tests/wpt/blob/256877037fa53c4c90e48e7171a69ea0cef0d3be/workers/modules/dedicated-worker-import-csp.html#L21

And the top-level module worker script will 'static import' its
descendants.
https://github.com/web-platform-tests/wpt/blob/master/workers/modules/resources/static-import-remote-origin-script-worker.sub.js

But due to the spec issue whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-csp.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-csp.html?label=experimental&label=master&aligned

So removing these two tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these two files from
interop-2023-modules completely)
nt1m pushed a commit to web-platform-tests/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh @nt1m
Remove [shared|dedicated]-worker-import-data-url-cross-origin.html fr…
de7e346 
…om interop-2023-modules

Fixing web-platform-tests/interop#406

In the [static import script from data: URL should be allowed.] case,
The worker will load the data URI, which imports "/resources/static-import-script-block-cross-origin.js".

https://github.com/web-platform-tests/wpt/blob/master/workers/modules/dedicated-worker-import-data-url-cross-origin.html#L13

And static-import-script-block-cross-origin.js will static import 'export-block-cross-origin.js'

https://github.com/web-platform-tests/wpt/blob/master/workers/modules/resources/static-import-script-block-cross-origin.js

The script "export-block-cross-origin.js" is same-origin with the
document, but not same-origin with the worker script which is loaded by
data: URI.

Due to the spec issue, whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url-cross-origin.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-data-url-cross-origin.html?label=experimental&label=master&aligned

So removing these two tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these two files from
interop-2023-modules completely)
nt1m pushed a commit to web-platform-tests/wpt-metadata that referenced this issue Sep 21, 2023
@allstarschh @nt1m
Remove dedicated-worker-import-data-url.any(.worker).html and shared-…
2727eae 
…worker-import-data-url.window.html from interop-2023-modules

Fixing web-platform-tests/interop#406

In this test, The worker will load a data URI, which import another test
script statically, and the imported script will import 'export-on-load-script.js',
which will be served with the CORS header 'Access-Control-Allow-Origin: *' in
/workers/modules/resources/export-on-load-script.js.headers

In the test case [Static import (redirect).], 'export-on-load-script.js' will be
redirected, and the redirected script doesn't have the CORS header.

https://github.com/web-platform-tests/wpt/blob/2cd449b9df6f92862abac0143ac6fe674fd5278e/workers/modules/resources/import-test-cases.js#L13
https://github.com/web-platform-tests/wpt/blob/2cd449b9df6f92862abac0143ac6fe674fd5278e/workers/modules/resources/static-import-redirect-worker.js#L3

The script "export-on-load-script.js" is same-origin with the
document, but not same-origin with the worker script which is loaded by
data: URI.

Due to the spec issue, whatwg/html#9571
There isn't an agreement between browser vendors on which [settings object](https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object)
to use when fetching the descendants of module worker.

https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url.any.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/dedicated-worker-import-data-url.any.worker.html?label=experimental&label=master&aligned
https://wpt.fyi/results/workers/modules/shared-worker-import-data-url.window.html?label=experimental&label=master&aligned

So removing these three tests from interop-2023-modules.
(According to the README.md in wpt-metadata, when a 'label' is used,
'subtest' is omitted, so I remove these three files from
interop-2023-modules completely)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
focus area: Modules test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update interop 2023 modules related to html issue 9571 allstarschh/wpt-metadata
6 participants
@hayatoito @allstarschh @foolip @nt1m @nairnandu @youennf