Closed Bug 1330900 Opened 4 years ago Closed 3 years ago

Implement <script nomodule>

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: annevk, Assigned: baku)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

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.
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
Priority: -- → P3
Blocks: 1344486
Assignee: nobody → amarchesini
Attached patch patchSplinter Review
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)
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.
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.
Attached patch patch WPTSplinter Review
Attachment #8864246 - Flags: review?(annevk)
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)
https://hg.mozilla.org/mozilla-central/rev/6cb894cff54d
https://hg.mozilla.org/mozilla-central/rev/ed5f27a0bf6c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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....
Thanks Boris, you hit the nail on the head with Bug 1382020.
I'm testing this now with "dom.moduleScripts.enabled" enabled in "about:config" but can see requests for both scripts in the Network tab.

```
<script type="module" src="module.js"></script>
<script nomodule src="fallback.js"></script>
```

```
404  GET  fallback.js
404  GET  module.js
```
Oops, I can see this has been "RESOLVED FIXED in Firefox 60" in Bug 1382020 above ^.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.