Implement <script nomodule>

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM
P3
normal
RESOLVED FIXED
7 months ago
a month ago

People

(Reporter: annevk, Assigned: baku)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
mozilla55
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 months ago
Discussion: https://github.com/whatwg/html/issues/1442.

Change: https://github.com/whatwg/html/pull/2261.

If for some reason we don't think this is a good idea now would be the time to object.
ccing some DOM folks just in case they have opinions.

Comment 2

7 months ago
Seems reasonable to me.  I think we should support efficient feature detection where possible and it should make it easier for modules to be adopted.
This seems good to me.

Dave
Keywords: dev-doc-needed

Updated

6 months ago
Priority: -- → P3

Updated

5 months ago
Blocks: 1344486
(Assignee)

Updated

4 months ago
Assignee: nobody → amarchesini
(Assignee)

Comment 4

4 months ago
Created attachment 8864231 [details] [diff] [review]
patch
Attachment #8864231 - Flags: review?(bzbarsky)
Comment on attachment 8864231 [details] [diff] [review]
patch

This needs an intent to implement so people can examine the idea, though I guess in practice we're not shipping this yet, right?

>+      scriptContent->IsHTMLElement() &&

Do we not want to support nomodule on <svg:script>?  I'd think we want to support it there.   Seems like something we should at least raise as an issue with ... yeah, I don't know who.  :(

>   // Step 14. in the HTML5 spec

That number is presumably off now.  Certainly the earlier "Step 12" in this function should now be "Step 13".  The "Step 14"... might now be "Step 15 and later" or something?

>+++ b/dom/html/test/file_script_module.html

I guess this can't be a web platform test (and the existing web platform tests aren't passing with this patch) because modules are not actually enabled by default anywhere so far?

r=me
Attachment #8864231 - Flags: review?(bzbarsky) → review+
Cameron, any idea where the right place is to raise the SVG <script> issue?
Flags: needinfo?(cam)
(Reporter)

Comment 7

4 months ago
According to Domenic there are web-platform-tests. And I don't see why we can't add any new tests there. The feature is in the HTML Standard already, which is the bar.
(Assignee)

Comment 8

4 months ago
My tests cannot be added to WPT because they checks the behavior of enabling/disabling the module support.
All the rest is already covered by WPT.
(Assignee)

Comment 9

4 months ago
Created attachment 8864246 [details] [diff] [review]
patch WPT
Attachment #8864246 - Flags: review?(annevk)
(Reporter)

Comment 10

4 months ago
Comment on attachment 8864246 [details] [diff] [review]
patch WPT

As far as I know these tests are correct, so if we're passing them that's good. From what I heard more tests are being added so we might have some more work going forward.
Attachment #8864246 - Flags: review?(annevk) → review+
> According to Domenic there are web-platform-tests. 

There are, but the point is that we don't actually support modules in our default configuration yet, so they're not necessarily useful for testing things.

I'm a bit leery of just forcing the pref on for wpt when we don't actually ship in that configuration...  Makes it too easy to forget to ever actually ship.  That said, we can probably do it in this case; presumably something already tracks shipping modules.
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #6)
> Cameron, any idea where the right place is to raise the SVG <script> issue?

https://github.com/w3c/svgwg/issues
Flags: needinfo?(cam)

Comment 13

4 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cb894cff54d
Implement <script nomodule>, r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed5f27a0bf6c
Enable WPT for <script nomodule>, r=annevk
Filed https://github.com/w3c/svgwg/issues/314

Comment 15

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6cb894cff54d
https://hg.mozilla.org/mozilla-central/rev/ed5f27a0bf6c
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I've documented this, adding a suitable entry to appropriate pages:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script
https://developer.mozilla.org/en-US/docs/Web/API/HTMLScriptElement

I've also added an entry to the Experimental features page:

https://developer.mozilla.org/en-US/Firefox/Experimental_features#HTML
Keywords: dev-doc-needed → dev-doc-complete

Comment 17

a month ago
First addition to bugzilla and not sure if this should be here or in a new report.

Using Firefox Developer Edition, with the dom.moduleScripts.enabled set to true, resources with nomodule attribute are still requested and transferred. When I looked at HTML spec is reads that the script should not be executed, but in asking the question, I was informed that the resources should not get requested.

I'm using 55.0b10 (64-bit)on Win10
dom.moduleScripts.enabled = true

not sure if you need any more info.
Robert, a new issue is generally better in cases like this.  I filed bug 1382020 with my best guess as to what it is you're actually seeing, but it's hard to tell without a link to a page where you're seeing the problem....

Comment 19

a month ago
Thanks Boris, you hit the nail on the head with Bug 1382020.
You need to log in before you can comment on or make changes to this bug.