Skip to content

Commit

Permalink
Handle negative ids in rrdom correctly
Browse files Browse the repository at this point in the history
When iframes get inserted they create untracked elements, both on the dom and rrdom side.
Because they are untracked they generate negative numbers when fetching the id from mirror.
This creates a problem when comparing and fetching ids across mirrors.
This commit tries to get away from using negative ids as much as possible in rrdom's comparisons
  • Loading branch information
Juice10 authored and Vadman97 committed Jul 1, 2022
1 parent 9a28a5d commit 563c111
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 10 deletions.
4 changes: 2 additions & 2 deletions packages/rrdom-nodejs/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@highlight-run/rrdom-nodejs",
"version": "0.1.2",
"version": "0.1.3",
"scripts": {
"dev": "rollup -c -w",
"bundle": "rollup --config",
Expand Down Expand Up @@ -46,7 +46,7 @@
"typescript": "^4.7.3"
},
"dependencies": {
"@highlight-run/rrdom": ">=0.1.12",
"@highlight-run/rrdom": ">=0.1.13",
"@highlight-run/rrweb-snapshot": ">=1.1.22",
"cssom": "^0.5.0",
"cssstyle": "^2.3.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/rrdom/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@highlight-run/rrdom",
"version": "0.1.12",
"version": "0.1.13",
"homepage": "https://github.com/rrweb-io/rrweb/tree/main/packages/rrdom#readme",
"license": "MIT",
"main": "lib/rrdom.js",
Expand Down
37 changes: 32 additions & 5 deletions packages/rrdom/src/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,37 +262,61 @@ function diffChildren(
let oldIdToIndex: Record<number, number> | undefined = undefined,
indexInOld;
while (oldStartIndex <= oldEndIndex && newStartIndex <= newEndIndex) {
const oldStartId = replayer.mirror.getId(oldStartNode);
const oldEndId = replayer.mirror.getId(oldEndNode);
const newStartId = rrnodeMirror.getId(newStartNode);
const newEndId = rrnodeMirror.getId(newEndNode);
console.log('oldStartId', oldStartId);
console.log('oldEndId', oldEndId);
console.log('newStartId', newStartId);
console.log('newEndId', newEndId);

// rrdom contains elements with negative ids, we don't want to accidentally match those to a mirror mismatch (-1) id.
// Negative oldStartId happen when nodes are not in the mirror, but are in the DOM.
// eg.iframes come with a document, html, head and body nodes.
// thats why below we always check if an id is negative.

if (oldStartNode === undefined) {
oldStartNode = oldChildren[++oldStartIndex];
} else if (oldEndNode === undefined) {
oldEndNode = oldChildren[--oldEndIndex];
} else if (
replayer.mirror.getId(oldStartNode) === rrnodeMirror.getId(newStartNode)
oldStartId !== -1 &&
// same first element?
oldStartId === newStartId
) {
diff(oldStartNode, newStartNode, replayer, rrnodeMirror);
oldStartNode = oldChildren[++oldStartIndex];
newStartNode = newChildren[++newStartIndex];
} else if (
replayer.mirror.getId(oldEndNode) === rrnodeMirror.getId(newEndNode)
oldEndId !== -1 &&
// same last element?
oldEndId === newEndId
) {
diff(oldEndNode, newEndNode, replayer, rrnodeMirror);
oldEndNode = oldChildren[--oldEndIndex];
newEndNode = newChildren[--newEndIndex];
} else if (
replayer.mirror.getId(oldStartNode) === rrnodeMirror.getId(newEndNode)
oldStartId !== -1 &&
// is the first old element the same as the last new element?
oldStartId === newEndId
) {
parentNode.insertBefore(oldStartNode, oldEndNode.nextSibling);
diff(oldStartNode, newEndNode, replayer, rrnodeMirror);
oldStartNode = oldChildren[++oldStartIndex];
newEndNode = newChildren[--newEndIndex];
} else if (
replayer.mirror.getId(oldEndNode) === rrnodeMirror.getId(newStartNode)
oldEndId !== -1 &&
// is the last old element the same as the first new element?
oldEndId === newStartId
) {
parentNode.insertBefore(oldEndNode, oldStartNode);
diff(oldEndNode, newStartNode, replayer, rrnodeMirror);
oldEndNode = oldChildren[--oldEndIndex];
newStartNode = newChildren[++newStartIndex];
} else {
// none of the elements matched

if (!oldIdToIndex) {
oldIdToIndex = {};
for (let i = oldStartIndex; i <= oldEndIndex; i++) {
Expand Down Expand Up @@ -368,8 +392,11 @@ export function createOrGetNode(
domMirror: NodeMirror,
rrnodeMirror: Mirror,
): Node {
let node = domMirror.getNode(rrnodeMirror.getId(rrNode));
const nodeId = rrnodeMirror.getId(rrNode);
const sn = rrnodeMirror.getMeta(rrNode);
let node: Node | null = null;
// negative ids shouldn't be compared accross mirrors
if (nodeId > -1) node = domMirror.getNode(nodeId);
if (node !== null) return node;
switch (rrNode.RRNodeType) {
case RRNodeType.Document:
Expand Down
106 changes: 106 additions & 0 deletions packages/rrdom/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,112 @@ describe('diff algorithm for rrdom', () => {
expect(element.nodeType).toBe(element.DOCUMENT_TYPE_NODE);
expect(mirror.getId(element)).toEqual(1);
});

it('should remove children from document before adding new nodes 2', () => {
document.write('<html><iframe></iframe></html>');

const iframe = document.querySelector('iframe')!;
// Remove everthing from the iframe but the root html element
// `buildNodeWithSn` injects docType elements to trigger compatMode in iframes
iframe.contentDocument!.write(
'<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "">',
);

replayer.mirror.add(iframe.contentDocument!, {
id: 1,
type: 0,
childNodes: [
{
id: 2,
rootId: 1,
type: 2,
tagName: 'html',
childNodes: [],
attributes: {},
},
],
} as serializedNodeWithId);
replayer.mirror.add(iframe.contentDocument!.childNodes[0], {
id: 2,
rootId: 1,
type: 2,
tagName: 'html',
childNodes: [],
attributes: {},
} as serializedNodeWithId);

const rrDocument = new RRDocument();
rrDocument.mirror.add(rrDocument, getDefaultSN(rrDocument, 1));
const docType = rrDocument.createDocumentType('html', '', '');
rrDocument.mirror.add(docType, getDefaultSN(docType, 2));
rrDocument.appendChild(docType);
const htmlEl = rrDocument.createElement('html');
rrDocument.mirror.add(htmlEl, getDefaultSN(htmlEl, 3));
rrDocument.appendChild(htmlEl);
const styleEl = rrDocument.createElement('style');
rrDocument.mirror.add(styleEl, getDefaultSN(styleEl, 4));
htmlEl.appendChild(styleEl);
const headEl = rrDocument.createElement('head');
rrDocument.mirror.add(headEl, getDefaultSN(headEl, 5));
htmlEl.appendChild(headEl);
const bodyEl = rrDocument.createElement('body');
rrDocument.mirror.add(bodyEl, getDefaultSN(bodyEl, 6));
htmlEl.appendChild(bodyEl);

diff(iframe.contentDocument!, rrDocument, replayer);
expect(iframe.contentDocument!.childNodes.length).toBe(2);
const element = iframe.contentDocument!.childNodes[0] as HTMLElement;
expect(element.nodeType).toBe(element.DOCUMENT_TYPE_NODE);
expect(mirror.getId(element)).toEqual(2);
});

it('should remove children from document before adding new nodes 3', () => {
document.write('<html><body><iframe></iframe></body></html>');

const iframeInDom = document.querySelector('iframe')!;

replayer.mirror.add(iframeInDom, {
id: 3,
type: 2,
rootId: 1,
tagName: 'iframe',
childNodes: [],
attributes: {},
} as serializedNodeWithId);
replayer.mirror.add(iframeInDom.contentDocument!, {
id: 4,
type: 0,
childNodes: [],
} as serializedNodeWithId);

const rrDocument = new RRDocument();

const rrIframeEl = rrDocument.createElement('iframe');
rrDocument.mirror.add(rrIframeEl, getDefaultSN(rrIframeEl, 3));
rrDocument.appendChild(rrIframeEl);
rrDocument.mirror.add(
rrIframeEl.contentDocument!,
getDefaultSN(rrIframeEl.contentDocument!, 4),
);

const rrDocType = rrDocument.createDocumentType('html', '', '');
rrIframeEl.contentDocument.appendChild(rrDocType);
const rrHtmlEl = rrDocument.createElement('html');
rrDocument.mirror.add(rrHtmlEl, getDefaultSN(rrHtmlEl, 6));
rrIframeEl.contentDocument.appendChild(rrHtmlEl);
const rrHeadEl = rrDocument.createElement('head');
rrDocument.mirror.add(rrHeadEl, getDefaultSN(rrHeadEl, 8));
rrHtmlEl.appendChild(rrHeadEl);
const bodyEl = rrDocument.createElement('body');
rrDocument.mirror.add(bodyEl, getDefaultSN(bodyEl, 9));
rrHtmlEl.appendChild(bodyEl);

diff(iframeInDom, rrIframeEl, replayer);
expect(iframeInDom.contentDocument!.childNodes.length).toBe(2);
const element = iframeInDom.contentDocument!.childNodes[0] as HTMLElement;
expect(element.nodeType).toBe(element.DOCUMENT_TYPE_NODE);
expect(mirror.getId(element)).toEqual(-1);
});
});

describe('create or get a Node', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@highlight-run/rrweb",
"version": "2.0.15",
"version": "2.0.16",
"description": "record and replay the web",
"scripts": {
"prepare": "npm run prepack",
Expand Down Expand Up @@ -74,7 +74,7 @@
"typescript": "^4.7.3"
},
"dependencies": {
"@highlight-run/rrdom": ">=0.1.12",
"@highlight-run/rrdom": ">=0.1.13",
"@highlight-run/rrweb-snapshot": ">=1.1.22",
"@types/css-font-loading-module": "0.0.7",
"@xstate/fsm": "^1.4.0",
Expand Down

0 comments on commit 563c111

Please sign in to comment.