1818447 - workers with { type: module} should not throw if module workers are disabled

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect, P3)

Comment 1

[daxpedda]

I am not sure what the new behavior will be, but currently worker module support is detected by checking if the type property is accessed at all.
See https://stackoverflow.com/questions/62954570/javascript-feature-detect-module-support-for-web-workers for example.

So if the new behavior will read the type property but not throw, current implementations might assume that type: module is supported.

Comment 2

[Yulia Startsev [:yulia]]

Thanks for the heads up. I tested the usecase there and it does work with the changes. Let me know if you run into any issues on beta.

Comment 3

[Yulia Startsev [:yulia]]

Attachment: Bug 1818447 - Remove references to type() if modules pref is off; r=jonco

Comment 4

[Yulia Startsev [:yulia]]

Attachment: Bug 1818447 - Update WPT beta tests to reflect changes in module flag; r=jonco

This patch updates several tests which were incorrectly updated with the flag caused attempts to
create modules to throw. Now accessing the type no longer happens, and should address web compat
concerns.

I was surprised by which tests were turned on, but I verified them against searchfox and the tests
that were updated as part of the workers work, and this seems correct.

Depends on D170920

Comment 6

[Donal Meehan [:dmeehan]]

Moving tracking from 1818732 to here.
:yulia, please nominate for beta uplift request when this lands in central and is good to uplift.
This week is the final beta week for 111, with the last beta build on Thursday 2022-03-02

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/803764b6d2b9
Remove references to type() if modules pref is off; r=jonco,webidl,smaug
https://hg.mozilla.org/integration/autoland/rev/faa460b11c9c
Update WPT beta tests to reflect changes in module flag; r=jonco

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on disabled-by-permissions-policy-cross-origin.https

• backout: https://hg.mozilla.org/integration/autoland/rev/5444176cc401026bd1493a1152153114cfcde089
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=faa460b11c9cf0310c740654e1b44174c7184b58
• failure log: https://treeherder.mozilla.org/logviewer?job_id=407189526&repo=autoland&lineNumber=22269

[task 2023-02-27T22:34:29.876Z] 22:34:29     INFO - TEST-FAIL | /web-share/disabled-by-permissions-policy-cross-origin.https.sub.html | share() is disabled explicitly by permissions policy for cross-origin iframe - assert_equals: expected "NotAllowedError" but got "Error"
[task 2023-02-27T22:34:29.876Z] 22:34:29     INFO - @https://web-platform.test:8443/web-share/disabled-by-permissions-policy-cross-origin.https.sub.html:66:20
[task 2023-02-27T22:34:29.876Z] 22:34:29     INFO - TEST-UNEXPECTED-FAIL | /web-share/disabled-by-permissions-policy-cross-origin.https.sub.html | share() not allowed, as only allowed to share with self - assert_equals: expected "NotAllowedError" but got "Error"
[task 2023-02-27T22:34:29.876Z] 22:34:29     INFO - @https://web-platform.test:8443/web-share/disabled-by-permissions-policy-cross-origin.https.sub.html:76:20
[task 2023-02-27T22:34:29.877Z] 22:34:29     INFO - ...
[task 2023-02-27T22:34:29.877Z] 22:34:29     INFO - TEST-OK | /web-share/disabled-by-permissions-policy-cross-origin.https.sub.html | took 2511ms

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/428c468be1f9
Remove references to type() if modules pref is off; r=jonco,webidl,smaug
https://hg.mozilla.org/integration/autoland/rev/a15b3da16984
Update WPT beta tests to reflect changes in module flag; r=jonco

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox111 (beta). We have limited time to fix this, the soft freeze is in 9 days. However, the bug still has low priority and has low severity.

:jstutte, could you please increase the priority and increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 11

[Yulia Startsev [:yulia]]

Attachment: Bug 1818447 - BETA UPLIFT: Update WPT beta tests; r=jonco

Comment 12

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/428c468be1f9
https://hg.mozilla.org/mozilla-central/rev/a15b3da16984

Comment 13

[Yulia Startsev [:yulia]]

Comment on attachment 9319720 [details]
Bug 1818447 - Remove references to type() if modules pref is off; r=jonco

Beta/Release Uplift Approval Request

• User impact if declined: There will be web compat breakage and some sites using workers will not work.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: This website should work without breakage https://web.dilato.app (see 1818732) and this script should return "false" on beta: https://stackoverflow.com/questions/62954570/javascript-feature-detect-module-support-for-web-workers
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): There is some risk that some webcompat is still missing, but this hopefully got everything.
• String changes made/needed:
• Is Android affected?: Yes

Comment 14

[Donal Meehan [:dmeehan]]

Comment on attachment 9319720 [details]
Bug 1818447 - Remove references to type() if modules pref is off; r=jonco

Approved for 111.0b8

Comment 15

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5eae4541365b
https://hg.mozilla.org/releases/mozilla-beta/rev/a0909e1e0365

Comment 16

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue with 111.0b7 on Windows 10x64. The https://web.dilato.app/ webpage is not loaded and the https://stackoverflow.com/questions/62954570/javascript-feature-detect-module-support-for-web-workers script returns true value.
The issue is verified fixed with Firefox 111.0b8 on Windows 10x64, macOS 12 and Ubuntu 20.04. The https://web.dilato.app/webpage is correctly opened and the script returns false value.
On Firefox Nightly 112.0a1 (2023-02-03) the https://web.dilato.app/ webpage is correctly opened but the returned valued when running the script is true. Is this the expected behavior for Nightly? Thank you in advance!

Comment 17

[Yulia Startsev [:yulia]]

On Firefox Nightly 112.0a1 (2023-02-03) the https://web.dilato.app/ webpage is correctly opened but the returned valued when running the script is true. Is this the expected behavior for Nightly? Thank you in advance!

yes, this is expected as module workers are turned on by default on nightly for testing and fuzzing. Thanks for the QA work!

Comment 18

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to Yulia Startsev [:yulia] from comment #17)

On Firefox Nightly 112.0a1 (2023-02-03) the https://web.dilato.app/ webpage is correctly opened but the returned valued when running the script is true. Is this the expected behavior for Nightly? Thank you in advance!

yes, this is expected as module workers are turned on by default on nightly for testing and fuzzing. Thanks for the QA work!

Thank you! Closing this per the above comments.