DOM: Added a few batch-insertion scenarios that are incompatible across engines #44658
Conversation
noamr
changed the title
There was a problem hiding this comment.
Mostly small stuff so far about test grouping and making the expectations more clear by using string/state variables instead of booleans that are equal to foo === null. Separately, I think it would be good to add test descriptions to each of the test(() => {}, "<description here>")s, just to make it more clear what each is asserting, and maybe add a little information about why.
dom/nodes/insertion-effects/Node-appendChild-iframe-before-script.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-button-from-div.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-custom-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-custom-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
...nsertion-effects/Node-appendChild-script-and-default-style-meta-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-iframe-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-iframe-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-iframe.tentative.html
Outdated
Show resolved
Hide resolved
...s/insertion-effects/Node-appendChild-script-and-stylesheet-link-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-stylesheet-link.tentative.html
Outdated
Show resolved
Hide resolved
| const iframe = document.createElement("iframe"); | ||
|
|
||
| test(() => { | ||
| iframe.src = "data:text/html,"; |
| test(() => { | ||
| window.state = "script not run yet"; | ||
| window.iframe = document.createElement("iframe"); | ||
| iframe.src = "data:text/html," |
There was a problem hiding this comment.
Same, is this important, or will the about:blank Document suffice?
|
|
||
| assert_array_equals(happened, []); | ||
| document.body.appendChild(df); | ||
| assert_array_equals(happened, [true]); |
There was a problem hiding this comment.
It depends on what we mean by "insertion effects". I don't think the goal is to eradicate all insertion side effects from taking place during DOM mutations. I think the goal is to eradicate all synchronous script-execution from the DOM mutation flow, to be queued until after all DOM mutations are done. The mutations themselves may have script-observable side effects by definition, which can be observed by the time earlier-inserted scripts run. (Although I'm not sure what we want to do about that in this case specifically).
dom/nodes/insertion-effects/Node-appendChild-script-and-style-from-fragment.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-and-style.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-script-in-script.tentative.html
Outdated
Show resolved
Hide resolved
dom/nodes/insertion-effects/Node-appendChild-text-and-script-in-style.tentative.html
Outdated
Show resolved
Hide resolved
|
(I've added a few changes, test description updates, and test coalescing in response to this and my prior review; feel free to check them out). |
dom/nodes/insertion-effects/Node-appendChild-script-before-iframe.tentative.html
Outdated
Show resolved
Hide resolved
The test follows the spec, but the spec is followed only by Safari.
This test too follows the spec, but only Safari defines the custom element before inserting one in the document, other browsers upgrade the custom-element at the time of the customElements.define call because it is already in tree.
This test once again follows the spec, but only Safari executes it correctly, with no form owner set on the button at the time of the execution of the script.
Safari and Chrome pass that test, Firefox fails it.
…-from-div.tentative.html Co-authored-by: Dominic Farolino <[email protected]>
…heet-link-from-fragment.tentative.html Co-authored-by: Dominic Farolino <[email protected]>
Co-authored-by: Dominic Farolino <[email protected]>
domfarolino
force-pushed
the
weird-insertion
branch
from
dc2c7ac
to
76b9cb2
Compare
|
I've rebased this on top of |
Repurposing #15264 to clean up the mess around deferred insertion steps.
Made the tests tentative, and cleaned up a few of the results.
Currently the expected results match Firefox.
They're marked tentative, and we can reason about the differences and fix the tests when we come to a conclusion.
Together with @annevk and @nox