fix (rrdom): Fix edge case where oldChild can become detach causing the recursive function to end early

[arivyiii]

Fix an edge case in diffChildren: when we replace an old child via oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); its nextSibling pointer is lost, causing the diff loop to exit early and inadvertently remove subsequent elements and styles.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4f74fe6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages

Name	Type
rrdom	Patch
rrweb	Patch
rrweb-snapshot	Patch
rrdom-nodejs	Patch
rrweb-player	Patch
@rrweb/all	Patch
@rrweb/replay	Patch
@rrweb/record	Patch
@rrweb/types	Patch
@rrweb/packer	Patch
@rrweb/utils	Patch
@rrweb/web-extension	Patch
rrvideo	Patch
@rrweb/rrweb-plugin-console-record	Patch
@rrweb/rrweb-plugin-console-replay	Patch
@rrweb/rrweb-plugin-sequential-id-record	Patch
@rrweb/rrweb-plugin-sequential-id-replay	Patch
@rrweb/rrweb-plugin-canvas-webrtc-record	Patch
@rrweb/rrweb-plugin-canvas-webrtc-replay	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pauldambra]

may be a silly question... where is handleInsertBefore defined?
i don't find it in rrweb at all

[eoghanmurray]

Yes, maybe this PR is incomplete? @arivyiii is that a mispelling or something that was left out?

[arivyiii]

Sorry I missed your messages. This PR was originally using handleInsertBefore (a safe wrapper around oldTree.insertBefore from another commit), but since oldTree.insertBefore works fine for this use case, I've updated the PR accordingly.

[eoghanmurray]

Your test case is very helpful.
I'm assuming the newList.splice(1, 0, { tagName: 'STYLE', id: 1 }); line is necessary to replicate the bug.
It is not clear to me how that scenario would arise 'in the wild'? Would it be possible to explain the motivating scenario a little more?

[arivyiii]

It is not clear to me how that scenario would arise 'in the wild'? Would it be possible to explain the motivating scenario a little more?

Yeah, it was a strange case that occurred in a few of our replays when users were using this extension: https://chromewebstore.google.com/detail/dark-reader/eimadpbcbfnmbkopoojfekhnkhdbieeh?hl=en-US. For some reason, the style tags were being appended with the same node ID back to back. I wasn’t able to reproduce it on any site with the recorder, but while debugging the replay I noticed that the childNode had a nextSibling, which was lost once it returned to diffChildren. We felt it was a safe enough change since for most replays they will never enter this block of code and if this edge case occur it would get resolved versus our users having to deal with a distorted replay.