1330657 - Enable <script type="module"> for content behind a pref

Status: RESOLVED FIXED

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

Description

[Tom S [:evilpie]]

We should let people experiment with modules. Hopefully we can actually ship milestone 0 soon. If CORS is the last thing blocking that I would suggest shipping with CORS first. Oh and look at what Safari and possibly Chrome are implementing.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Worth sending an "intent to experiment" or something, possibly.

I'm not seeing an obvious bug on the CORS thing in the dep tree of bug 568953.  Is that filed?

Comment 2

[Jon Coppeard (:jonco)]

The current plan is to enable this behind a pref.  This will allow easier testing with web platform tests too.

Comment 3

[Jon Coppeard (:jonco)]

I sent an "Intent to implement" to dev.platform a few weeks ago:

https://groups.google.com/forum/#!topic/mozilla.dev.platform/dr67sITwva8

Comment 4

[Jon Coppeard (:jonco)]

Attachment: bug1330657-enable-with-pref

Patch to enable modules with dom.moduleScripts.enabled pref.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8834912 [details] [diff] [review]
bug1330657-enable-with-pref

Review of attachment 8834912 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsScriptLoader.cpp
@@ +658,5 @@
>  bool
> +nsScriptLoader::ModuleScriptsEnabled()
> +{
> +  return nsContentUtils::IsChromeDoc(mDocument) ||
> +         mozilla::Preferences::GetBool("dom.moduleScripts.enabled", false);

Do you want to use Preferences::AddBoolVarCache() instead since it looks like this gets consulted on every script load?  I'm not sure how slow GetBool() itself is.

Comment 6

[Boris Zbarsky [:bzbarsky]]

GetBool is mostly a hashtable lookup and a bunch of method calls.  But yes, using a bool var cache here would be much better.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Also, no need for that "mozilla::" noise in this file.

Comment 8

[Jon Coppeard (:jonco)]

Great, I'll use that then.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/effda19da482
Enable <script type="module"> behind a pref r=bkelly

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/effda19da482

Comment 11

[Chris Mills [:cmills]]

I've added a note about this to our Experimental features page:

https://developer.mozilla.org/en-US/Firefox/Experimental_features

I've removed the mention that had been added to the Fx54 release notes, as it is not usually our policy to mention features there until they are enabled by default, afaiu.

Let me know if that looks OK.