Re: [whatwg/dom] Fix NodeIterator pre-remove steps for ancestors of root (PR #1477)

shannonbooth left a comment (whatwg/dom#1477)

This does make the meat of the NodeIterator implementation match, but I believe we have one extra deviation in place to match the behaviour of other engines. Apparently this is actually done by Acid3 where it was originally found: https://github.com/web-platform-tests/wpt/blob/b62d7567fd2df9b5a40788a2e9a42f9de756fc21/acid/acid3/test.html#L297

Claude says this so I could try and get a summary:

> Under the literal spec, the filter callback in step 3.2 runs user JS that may remove nodes, and those pre-removing steps adjust the iterator's stored reference. But traverse is working on its local copy node, which the pre-removing steps can't touch — and then steps 4–5 overwrite the stored reference with that local anyway. So a literal implementation would (a) keep advancing from a node that may now be detached, and (b) clobber any retargeting the pre-removing steps performed.

> Fix: Have traverse operate on the iterator's live state instead of a snapshot, while still returning the node handed to the filter.

Here's a test I got it to make (for some reason it doesn't look like there is one in our tree):

```html
<script>
    test(() => {
        // Tree: root > [a, b, c]
        const root = document.createElement("div");
        const a = document.createElement("a-el");
        const b = document.createElement("b-el");
        const c = document.createElement("c-el");
        root.append(a, b, c);

        let removedDuringFilter = false;
        const it = document.createNodeIterator(root, NodeFilter.SHOW_ELEMENT, {
            acceptNode(node) {
                // Remove the node from the tree while it is being filtered, i.e. while
                // a traverse() is in flight. This fires the NodeIterator pre-removing
                // steps against the in-progress traversal pointer.
                if (node === b) {
                    b.remove();
                    removedDuringFilter = true;
                }
                return NodeFilter.FILTER_ACCEPT;
            }
        });

        // Walk down to `a`.
        println("1: " + it.nextNode().nodeName);
        println("2: " + it.nextNode().nodeName);

        // This advances to `b` and filters it; the filter removes `b`.
        const returned = it.nextNode();
        println("removed during filter: " + removedDuringFilter);

        // The value returned must be the node that was passed to the filter (`b`),
        // even though the traversal pointer was retargeted away from it.
        println("3 returned: " + (returned ? returned.nodeName : "null"));

        // The reference node must be retargeted to a still-attached node (`a`),
        // not left pointing at the removed `b`.
        println("3 referenceNode: " + it.referenceNode.nodeName);

        // Traversal continues sanely from the retargeted reference, reaching `c`
        // rather than getting stuck on the detached `b`.
        const next = it.nextNode();
        println("4 returned: " + (next ? next.nodeName : "null"));
    });
</script>

```

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

Message ID: <whatwg/dom/pull/1477/c4863431399@github.com>