Re: [whatwg/dom] Side effects due to tree insertion or removal (script, iframe) (#808)

> I believe the intent is to define exactly when during the insertion or removal the side effects happen. For example, if you insert a subtree into the DOM that contains two <script> tags, we need to define when exactly those run and what the state of the DOM looks like when they do. Right now the spec is not very clear on that.

It surprised me to read this. It's possible things in the spec have seriously changed for the better in the intervening years, but I always understood HTML/DOM to be pretty explicit about when insertion/removal side-effects happen. For example, DOM defines the [insertion steps](https://dom.spec.whatwg.org/#concept-node-insert-ext) hook, which HTML [overrides](https://html.spec.whatwg.org/C#dom-trees:concept-node-insert-ext). Furthermore, HTML provides a per-element [HTML element insertion steps](https://html.spec.whatwg.org/C#html-element-insertion-steps) hook that runs during HTML's own "insertion steps" for any element that overrides said hook. HTML also provides more granular hooks for acting on the narrow case when an element is "[inserted into a document](https://html.spec.whatwg.org/C#insert-an-element-into-a-document)", and the wider/general case of when an element "[becomes connected](https://html.spec.whatwg.org/C#becomes-connected)".

These hooks should pretty clearly define the ordering of insertion/removal side-effects, especially for simple elements that don't do a lot during these hooks. But even for complicated cases like in the OP, or with multiple iframe removals (as @annevk mentioned), I think the spec is at least clear on what it calls for.

### Removing multiple iframes

From https://github.com/whatwg/dom/issues/808#issuecomment-1344051386:

> The main question for this issue is what's the state of the world if you remove multiple iframe elements at once and if you get to observe the intermediate states.

I think the spec is clear on this, if for no other reason than in the intervening years the [navigation and session history rewrite](https://github.com/whatwg/html/commit/0a97a81da77bc4cb0ab5b16420605ca001ff5b17) has landed, and follow-ups like https://github.com/whatwg/html/pull/9907 have made things even better. If you have a document with two sibling iframes, and a script that calls `remove()` on them back-to-back in the same task, you'd get the following flow twice. (In **bold** should be every place where the spec allows script to run, and thus some observation of the current state.)

 - [ChildNode#remove()](https://dom.spec.whatwg.org/#dom-childnode-remove) → [remove algorithm](https://dom.spec.whatwg.org/#concept-node-remove)
 - [Removing steps](https://dom.spec.whatwg.org/#ref-for-concept-node-remove-ext) hook
 - [HTML element removing steps for iframes](https://html.spec.whatwg.org/C#the-iframe-element:html-element-removing-steps)
    - [Destroy a document and its descendants](https://html.spec.whatwg.org/C#destroy-a-document-and-its-descendants), in parallel (asynchronously jumping back to the main thread to do things)
    - [**Queue a "remove" mutation record on document.body with the iframe**](https://dom.spec.whatwg.org/#ref-for-queue-a-tree-mutation-record%E2%91%A3)

The spec is equally clear for cases with nested iframes and so on, as calls to the "removing steps" are [properly recursive](https://dom.spec.whatwg.org/#ref-for-concept-node-remove-ext%E2%91%A0). So spec-wise, I don't think there should be a way to observe the "intermediate state" at all when all removes happen synchronously with respect to each other. However, implementations seem to all disagree with the spec in the same way!

**Chrome, Firefox, Safari**
All browsers fire unload events synchronously "near" the [destroy a document and its descendants](https://html.spec.whatwg.org/C#destroy-a-document-and-its-descendants) algorithm (before `remove()` returns). This is not only inconsistent with HTML, which doesn't do this, but is totally opposite of [this note](https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:destroy-a-child-navigable:~:text=This%20happens%20without%20any%20unload%20events%20firing%20(the%20element%27s%20content%20document%20is%20destroyed%2C%20not%20unloaded).), which specifically says that `unload` events do not fire in this path. Tagging @domenic just in case he ran into any of this during the recent cleanups there.

This is kinda weird, but I don't think it's "broken", and FWIW all browsers are dead consistent. Here's what they can observe in the unload handler "mid-removal":

 - `window.parent != null` (i.e., `window.parent.postMessage()` still works)
    - I can't tell if this is called for by the spec; that is, I can't tell what [#dom-parent](https://html.spec.whatwg.org/multipage/nav-history-apis.html#dom-parent) should return in this state. During child destruction, but in parallel, we [clear the active SHE's document](https://html.spec.whatwg.org/C#destroying-documents:nav-active-history-entry), which I think is the only thing that would make `this's navigable` return null, and thus `window.parent` return null. But that's in parallel, so I don't think the spec actually does anything to clear the parent immediately. So I think the spec is technically OK here.
 - `window.frameElement == null`
    - This is covered by HTML. `frameElement`'s nullness is dependent on whether the iframe's content navigable [has a container](https://html.spec.whatwg.org/multipage/nav-history-apis.html#navigating-nested-browsing-contexts-in-the-dom%3Anav-container). Yet synchronously during the [iframe element removal steps](https://html.spec.whatwg.org/C#the-iframe-element:html-element-removing-steps) (which call "destroy a child navigable), the container's content navigable is [nulled out](https://html.spec.whatwg.org/C#garbage-collection-and-browsing-contexts:content-navigable-2), thus severing ties between the two navigables. This should make `frameElement` return null ✅. The only oddity is that `unload` _also_ happens to fire synchronously somewhere in the middle of "destroy a child navigable" algorithm. All browsers happen to fire `unload` after the container's content navigable is nulled out, which is why they are all consistent with `frameElement == null`.

But this `unload` issue seems like a scoped HTML-navigable-destruction bug, and less about the wider hooks in the overall DOM world that I would expect to block https://github.com/whatwg/html/issues/5484. If @domenic agrees I'll file something on HTML.

### Inserted DocumentFragment with two scripts

Consider the example in the OP. This is the flow the spec follows (again, **bold** is where (queued) scripts will run):

 - https://dom.spec.whatwg.org/#concept-node-insert
 - [**Queue a "remove" mutation record on fragment with all its children**](https://dom.spec.whatwg.org/#ref-for-queue-a-tree-mutation-record) (both scripts)
 - [Run insertion steps on each <script> element](https://dom.spec.whatwg.org/#ref-for-concept-node-insert-ext) (x2)
    - Causes the [HTML element insertion steps](https://html.spec.whatwg.org/C#html-element-insertion-steps) to run
    - Triggers the [becomes connected hook for script elements](https://html.spec.whatwg.org/C#script-processing-model:becomes-connected)
    - UAs must immediately [prepare the script element](https://html.spec.whatwg.org/C#prepare-the-script-element)
    - [Create a classic script](https://html.spec.whatwg.org/C#script-processing-model:creating-a-classic-script)
    - [Immediately execute the script](https://html.spec.whatwg.org/C#script-processing-model:execute-the-script-element-4)
        - Cannot observe `<script>` elements that will be inserted after it
 - [**Queue an "add" mutation record on document with fragment's children**](https://dom.spec.whatwg.org/#ref-for-queue-a-tree-mutation-record%E2%91%A0) (both scripts)

Only Safari follows the spec, and Chrome and Firefox behave exactly how the OP describes. So there is a clear interop issue, but the standard at least makes it clear which implementation is right vs. wrong. The question is: is the spec + Safari being aligned sufficient to simply call these "Chrome and Firefox bugs," or do we want to adjust the standard? I think the spec + Safari behavior is closer to what we all want, so maybe we can just file some Chrome + Firefox bugs, but I'd love to know what @annevk thinks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/808#issuecomment-1913887358
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/dom/issues/808/1913887358@github.com>