From 563c1119c1eb935ab8dd07b1893143e5b2e4e59d Mon Sep 17 00:00:00 2001 From: Justin Halsall Date: Fri, 1 Jul 2022 18:31:00 +0200 Subject: [PATCH] Handle negative ids in rrdom correctly 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 --- packages/rrdom-nodejs/package.json | 4 +- packages/rrdom/package.json | 2 +- packages/rrdom/src/diff.ts | 37 ++++++++-- packages/rrdom/test/diff.test.ts | 106 +++++++++++++++++++++++++++++ packages/rrweb/package.json | 4 +- 5 files changed, 143 insertions(+), 10 deletions(-) diff --git a/packages/rrdom-nodejs/package.json b/packages/rrdom-nodejs/package.json index b51a20d1..a0fc6b47 100644 --- a/packages/rrdom-nodejs/package.json +++ b/packages/rrdom-nodejs/package.json @@ -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", @@ -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", diff --git a/packages/rrdom/package.json b/packages/rrdom/package.json index ee329fc5..21dc82fb 100644 --- a/packages/rrdom/package.json +++ b/packages/rrdom/package.json @@ -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", diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 3de6fe0f..58ee6575 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -262,37 +262,61 @@ function diffChildren( let oldIdToIndex: Record | 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++) { @@ -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: diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 4c0a83ea..ec78cac1 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -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(''); + + 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( + '', + ); + + 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(''); + + 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', () => { diff --git a/packages/rrweb/package.json b/packages/rrweb/package.json index dabe7da1..33e2b816 100644 --- a/packages/rrweb/package.json +++ b/packages/rrweb/package.json @@ -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", @@ -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",