957054 - Don't preload scripts with unknown types

Status: NEW

(Core :: DOM: HTML Parser, task, P3)

Description

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

See
https://code.google.com/p/chromium/issues/detail?id=329531

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note discussion at https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eu57SCNltls

Comment 2

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, 4 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.

Comment 3

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Chrome has fixed their corresponding bug: https://bugs.chromium.org/p/chromium/issues/detail?id=626321

Comment 4

[yanshrey]

If chrome as fixed their corresponding bug does it also gets fixed on mozilla? If not then i would like to work on this bug.

Comment 5

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

(In reply to yanshrey from comment #4)

If not then i would like to work on this bug.

That would be great. See https://searchfox.org/mozilla-central/rev/1ebc9745be02eebf7a694f5c527a44a045b8c97a/parser/html/nsHtml5TreeBuilderCppSupplement.h#207 for the starting point.

Comment 6

[james]

Hi,
Is anyone still working on this bug?
If not want to give it a shot

Comment 7

[yanshrey]

I was looking into but but you can also go ahead and try it.
Check comment #5

Comment 8

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Fixing bug 1848311 would have the side effect of fixing this.

Comment 10

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Not only should we not preload unknown type, we should not create speculations when seeing unknown types.

From the bug I marked as duplicate:

We should check for IE event handler scripts and unsupported type/language combinations in the HTML parser already (not just in ScriptLoader) to avoid creating speculations or preloads for scripts that will not execute anyway.

Comment 11

[Mathew Hodson]

Attachment: WIP: Bug 957054 - Don't preload scripts with unknown type

Based on
https://html.spec.whatwg.org/multipage/scripting.html#prepare-a-script

This affects mSpeculativeLoadQueue. I didn't change the behavior of
mCurrentHtmlScriptCannotDocumentWriteOrBlock.

Comment 12

[Mathew Hodson]

I'm currently stuck trying to implement your suggestion to handle ScriptKind::eEvent like mozilla::dom::IsScriptEventHandler does. There isn't an nsHtml5AttributeName::ATTR_EVENT that I would need to detect an event attribute on a <script> element.

Comment 13

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

Sorry about the delay. Indeed, ATTR_EVENT is needed and missing. I'm adding it in bug 1884108.

Comment 14

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

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 15

[Mathew Hodson]

@smaug do you think you could land this for me? Or do you think it shouldn't land unless we add a new wpt for it?

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

I think this could use a test, if just possible. The change isn't trivial.

Comment 17

[Mathew Hodson]

Attachment: Bug 957054 - Test that unsupported script types don't speculatively load r=smaug

Comment 18

[Mathew Hodson]

I modified some existing WPT that check for speculative load. Can you land this for me if everything looks good?

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Which patches need landing? The stack in phabricator has 3 patches

Comment 20

[Mathew Hodson]

All three need to land. First the two for this bug 957054, and then the one in bug 1975128 since it depends on the changes in IsScriptEventHandler

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/78e1e21e6e3b
https://hg.mozilla.org/integration/autoland/rev/d67dd206a3f6
Don't preload scripts with unknown type r=smaug
https://github.com/mozilla-firefox/firefox/commit/f1d8923b7fe0
https://hg.mozilla.org/integration/autoland/rev/86e60753f501
Test that unsupported script types don't speculatively load r=smaug

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Hopefully it worked :)

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53999 for changes under testing/web-platform/tests

Comment 24

[pstanciu]

Backed out for causing BP bustages @ScriptLoader.cpp

• https://hg.mozilla.org/integration/autoland/rev/c58db972dcced4455e804ed530064a6a5014c27f
• Push with failures
• Failure Log

Comment 25

[Mathew Hodson]

Used explicit namespaces to fix nonunified build errors. Verified with ac_add_options --disable-unified-build and I didn't see any other failures in the treeherder link.

Could you land this again?

Comment 26

[[e-mail]]

https://hg.mozilla.org/mozilla-central/rev/d67dd206a3f6
https://hg.mozilla.org/mozilla-central/rev/86e60753f501

Comment 27

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 28

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 29

[Cristina Horotan [:chorotan]]

Backed out here: https://hg-edge.mozilla.org/integration/autoland/rev/c58db972dcced4455e804ed530064a6a5014c27f

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d67dd206a3f6
https://hg.mozilla.org/mozilla-central/rev/86e60753f501

Comment 31

[Mathew Hodson]

(In reply to Cristina Horotan [:chorotan] from comment #29)

Backed out here: https://hg-edge.mozilla.org/integration/autoland/rev/c58db972dcced4455e804ed530064a6a5014c27f

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e892f62139ef
https://hg.mozilla.org/integration/autoland/rev/6a3eb2f7b3b1
Don't preload scripts with unknown type r=smaug
https://github.com/mozilla-firefox/firefox/commit/2da909cbae2a
https://hg.mozilla.org/integration/autoland/rev/3179ff0b5ccb
Test that unsupported script types don't speculatively load r=smaug

Comment 33

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 34

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/6a3eb2f7b3b1
https://hg.mozilla.org/mozilla-central/rev/3179ff0b5ccb

Comment 35

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/54068 for changes under testing/web-platform/tests

Comment 36

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b5a01df6d65f
https://hg.mozilla.org/integration/autoland/rev/26331ad8f0c8
Revert "Bug 957054 - Test that unsupported script types don't speculatively load r=smaug" on request by smaug

Comment 38

[Cristina Horotan [:chorotan]]

Backed out on request https://hg.mozilla.org/integration/autoland/rev/26331ad8f0c8c57becc304f17975d6b3085c4d0c

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9c44d5da4225
https://hg.mozilla.org/releases/mozilla-beta/rev/2e0c099f88fc
Revert "Bug 957054 - Test that unsupported script types don't speculatively load r=smaug" on request by smaug

Comment 40

[Mathew Hodson]

Should I update the patch to do the more narrow change that's just for mSpeculativeLoadQueue and not change the way mCurrentHtmlScriptCannotDocumentWriteOrBlock is set?

Comment 41

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

(In reply to Mathew Hodson from comment #40)

Should I update the patch to do the more narrow change that's just for mSpeculativeLoadQueue and not change the way mCurrentHtmlScriptCannotDocumentWriteOrBlock is set?

Rather than generating two kinds of tree ops based on boolean mCurrentHtmlScriptCannotDocumentWriteOrBlock: execute script that may document.write or block and execute script that cannot document.write or block, there should be a third category: Scripts that must not be executed, so that the HTML parser's classification decision is final and the script execution code path doesn't get an opportunity to disagree.

We already have opPreventScriptExecution, but it doesn't perform a microtask checkpoint. From reading the spec, it looks to me that a new tree op opPreventScriptExecutionWithMicrotaskCheckpoint is needed. (When the spec pops a script off the stack at EOF, it prevents execution without a microtask checkpoint. However, when a script end tags is processed, a microtask checkpoint is performed before looking at the scripts type, etc.)

Comment 42

[Henri Sivonen (:hsivonen) (not reading bugmail until 2026-07-27)]

(That is, let's not simply change mSpeculativeLoadQueue, because the path for executing a script that may document.write or block is expensive, so if we know we don't want to execute the script, it makes sense to do something less expensive. We just need the decision to be final.)

Comment 43

[Iulian Moraru]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/26331ad8f0c8

Comment 44

[Mathew Hodson]

(In reply to Henri Sivonen (:hsivonen) from comment #41)

We already have opPreventScriptExecution, but it doesn't perform a microtask checkpoint. From reading the spec, it looks to me that a new tree op opPreventScriptExecutionWithMicrotaskCheckpoint is needed. (When the spec pops a script off the stack at EOF, it prevents execution without a microtask checkpoint. However, when a script end tags is processed, a microtask checkpoint is performed before looking at the scripts type, etc.)

Are there existing tests in this area I could look at? Sounds like we need a new test that checks script execution, and the WPTs I added for speculative fetching aren't sufficient.

Comment 45

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/54752 for changes under testing/web-platform/tests

Comment 46

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 47

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

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.