capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.28k stars 200 forks source link

TreeWalker `nextSibling`/`previousSibling` throw error when `currentNode` is set to another DOM tree #558

Closed maxmilton closed 1 year ago

maxmilton commented 2 years ago

When TreeWalker is walking a DOM tree it was not initialised with it throws an error when nextSibling/previousSibling are called and it reaches the (other DOM tree's) root.

Although the spec doesn't explicitly allow or deny it, it's actually possible to initialize a TreeWalker instance and then use the .currentNode setter to change to a completely different DOM tree and then walk it.

Given the following reproduction:

const { GlobalWindow } = require("happy-dom");

global.window = new GlobalWindow();
global.document = global.window.document;

const tree1 = document.createElement("div");
const tree2 = document.createElement("div");
const walker = document.createTreeWalker(tree1);

walker.currentNode = tree2;
walker.nextNode();

Running the code, this error is throw:

> happydom-tree-walker-repro@0.0.0 start
> node index.js

/home/max/Projects/_experiments/happydom-tree-walker-repro/node_modules/happy-dom/lib/tree-walker/TreeWalker.js:130
            const siblings = this.currentNode.parentNode.childNodes;
                                                         ^

TypeError: Cannot read properties of null (reading 'childNodes')
    at TreeWalker.nextSibling (/home/max/Projects/_experiments/happydom-tree-walker-repro/node_modules/happy-dom/lib/tree-walker/TreeWalker.js:130:58)
    at TreeWalker.nextNode (/home/max/Projects/_experiments/happydom-tree-walker-repro/node_modules/happy-dom/lib/tree-walker/TreeWalker.js:41:26)
    at Object.<anonymous> (/home/max/Projects/_experiments/happydom-tree-walker-repro/index.js:11:8)
    at Module._compile (node:internal/modules/cjs/loader:1120:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1174:10)
    at Module.load (node:internal/modules/cjs/loader:998:32)
    at Module._load (node:internal/modules/cjs/loader:839:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47

Node.js v18.7.0

I ~abuse~ use this behaviour in one of my libraries so I can reuse the same TreeWalker instance as a performance optimisation. Works as expected in browsers and jsdom, it's just happy-dom which throws.

capricorn86 commented 1 year ago

Thanks for contributing @maxmilton! 🙂

The fix you did has been released.

You can read more about the release here: https://github.com/capricorn86/happy-dom/releases/tag/v7.0.3