Closed
Bug 1330657
Opened 8 years ago
Closed 8 years ago
Enable <script type="module"> for content behind a pref
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: evilpies, Assigned: jonco)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, DevAdvocacy)
Attachments
(1 file)
4.13 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 1•8 years ago
|
||
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?
Updated•8 years ago
|
Keywords: DevAdvocacy
Assignee | ||
Updated•8 years ago
|
Component: JavaScript Engine → DOM: Core & HTML
Assignee | ||
Comment 2•8 years ago
|
||
The current plan is to enable this behind a pref. This will allow easier testing with web platform tests too.
Assignee: nobody → jcoppeard
Summary: Enable <script type="module"> for content on Nightly → Enable <script type="module"> for content behind a pref
Assignee | ||
Comment 3•8 years ago
|
||
I sent an "Intent to implement" to dev.platform a few weeks ago:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/dr67sITwva8
Assignee | ||
Comment 4•8 years ago
|
||
Patch to enable modules with dom.moduleScripts.enabled pref.
Attachment #8834912 -
Flags: review?(bkelly)
Comment 5•8 years ago
|
||
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.
Attachment #8834912 -
Flags: review?(bkelly) → review+
Comment 6•8 years ago
|
||
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•8 years ago
|
||
Also, no need for that "mozilla::" noise in this file.
Assignee | ||
Comment 8•8 years ago
|
||
Great, I'll use that then.
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/effda19da482
Enable <script type="module"> behind a pref r=bkelly
Comment 10•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 11•8 years ago
|
||
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•