From 213848cb17a196e21cdb47aeaeaa38d95d4ff793 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Tue, 10 Jan 2023 18:07:07 +1100 Subject: [PATCH 1/9] add more check to rrdom to make diff algorithm more robust --- packages/rrdom/src/diff.ts | 27 +++++- packages/rrdom/test/diff.test.ts | 136 ++++++++++++++++++++++++++++++- 2 files changed, 158 insertions(+), 5 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index f03557e7f5..397fc10348 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -93,12 +93,22 @@ export function diff( replayer: ReplayerHandler, rrnodeMirror?: Mirror, ) { - const oldChildren = oldTree.childNodes; - const newChildren = newTree.childNodes; rrnodeMirror = rrnodeMirror || (newTree as RRDocument).mirror || (newTree.ownerDocument as RRDocument).mirror; + // If the Mirror data has some flaws, the diff function may throw errors. We check the node consistency here to make it robust. + if (!sameNodeType(oldTree, newTree)) { + const calibratedOldTree = createOrGetNode( + newTree, + replayer.mirror, + rrnodeMirror, + ); + oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); + oldTree = calibratedOldTree; + } + const oldChildren = oldTree.childNodes; + const newChildren = newTree.childNodes; if (oldChildren.length > 0 || newChildren.length > 0) { diffChildren( @@ -417,7 +427,7 @@ export function createOrGetNode( 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; + if (node !== null && sameNodeType(node, rrNode)) return node; switch (rrNode.RRNodeType) { case RRNodeType.Document: node = new Document(); @@ -451,3 +461,14 @@ export function createOrGetNode( if (sn) domMirror.add(node, { ...sn }); return node; } + +/** + * To check whether two nodes are the same type of node. If they are both Elements, check wether their tagNames are same or not. + */ +export function sameNodeType(node1: Node, node2: IRRNode) { + if (node1.nodeType !== node2.nodeType) return false; + return ( + node1.nodeType !== node1.ELEMENT_NODE || + (node1 as HTMLElement).tagName === (node2 as IRRElement).tagName + ); +} diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 6e02577f3a..0918335425 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -2,14 +2,19 @@ * @jest-environment jsdom */ import { getDefaultSN, RRDocument, RRMediaElement } from '../src'; -import { createOrGetNode, diff, ReplayerHandler } from '../src/diff'; +import { + createOrGetNode, + diff, + ReplayerHandler, + sameNodeType, +} from '../src/diff'; import { NodeType as RRNodeType, serializedNodeWithId, createMirror, Mirror, } from 'rrweb-snapshot'; -import type { IRRNode } from '../src/document'; +import type { IRRElement, IRRNode } from '../src/document'; import { Replayer } from 'rrweb'; import type { eventWithTime, @@ -247,6 +252,29 @@ describe('diff algorithm for rrdom', () => { expect(element.paused).toEqual(true); } }); + + it('should diff a node with different node type', () => { + // When the diff target has a different node type. + let parentNode: Node = document.createElement('div'); + let unreliableNode: Node = document.createTextNode(''); + parentNode.appendChild(unreliableNode); + const rrNode = new RRDocument().createElement('li'); + diff(unreliableNode, rrNode, replayer); + expect(parentNode.childNodes.length).toEqual(1); + expect(parentNode.childNodes[0]).toBeInstanceOf(HTMLElement); + expect((parentNode.childNodes[0] as HTMLElement).tagName).toEqual('LI'); + + // When the diff target has the same node type but with different tagName. + parentNode = document.createElement('div'); + unreliableNode = document.createElement('span'); + parentNode.appendChild(unreliableNode); + diff(unreliableNode, rrNode, replayer); + expect((parentNode.childNodes[0] as HTMLElement).tagName).toEqual('LI'); + + // When the diff target is a node without parentNode. + unreliableNode = document.createComment(''); + diff(unreliableNode, rrNode, replayer); + }); }); describe('diff properties', () => { @@ -1001,6 +1029,59 @@ describe('diff algorithm for rrdom', () => { newElementsIds, ); }); + + it('should diff children with unreliable Mirror', () => { + const parentNode = createTree( + { + tagName: 'div', + id: 0, + children: [], + }, + undefined, + mirror, + ) as Node; + // Construct unreliable Mirror data. + const unreliableChild = document.createTextNode(''); + const unreliableSN = { + id: 1, + textContent: '', + type: RRNodeType.Text, + } as serializedNodeWithId; + mirror.add(unreliableChild, unreliableSN); + parentNode.appendChild(unreliableChild); + mirror.add(document.createElement('div'), { ...unreliableSN, id: 2 }); + + const rrParentNode = createTree( + { + tagName: 'div', + id: 0, + children: [1].map((c) => ({ + tagName: 'span', + id: c, + children: [2].map((c1) => ({ + tagName: 'li', + id: c1, + })), + })), + }, + new RRDocument(), + ) as RRNode; + const id = 'correctElement'; + (rrParentNode.childNodes[0] as IRRElement).setAttribute('id', id); + diff(parentNode, rrParentNode, replayer); + + expect(parentNode.childNodes.length).toEqual(1); + expect(parentNode.childNodes[0]).toBeInstanceOf(HTMLElement); + + const spanChild = parentNode.childNodes[0] as HTMLElement; + expect(spanChild.tagName).toEqual('SPAN'); + expect(spanChild.id).toEqual(id); + expect(spanChild.childNodes.length).toEqual(1); + expect(spanChild.childNodes[0]).toBeInstanceOf(HTMLElement); + + const liChild = spanChild.childNodes[0] as HTMLElement; + expect(liChild.tagName).toEqual('LI'); + }); }); describe('diff shadow dom', () => { @@ -1431,4 +1512,55 @@ describe('diff algorithm for rrdom', () => { ).toEqual('a {color: blue;}'); }); }); + + describe('test sameNodeType function', () => { + const rrdom = new RRDocument(); + it('should return true when two elements have same tagNames', () => { + const div1 = document.createElement('div'); + const div2 = rrdom.createElement('div'); + expect(sameNodeType(div1, div2)).toBeTruthy(); + }); + + it('should return false when two elements have different tagNames', () => { + const div1 = document.createElement('div'); + const div2 = rrdom.createElement('span'); + expect(sameNodeType(div1, div2)).toBeFalsy(); + }); + + it('should return false when two nodes have the same node type', () => { + let node1: Node = new Document(); + let node2: IRRNode = new RRDocument(); + expect(sameNodeType(node1, node2)).toBeTruthy(); + + node1 = document.implementation.createDocumentType('html', '', ''); + node2 = rrdom.createDocumentType('', '', ''); + expect(sameNodeType(node1, node2)).toBeTruthy(); + + node1 = document.createTextNode('node1'); + node2 = rrdom.createTextNode('node2'); + expect(sameNodeType(node1, node2)).toBeTruthy(); + + node1 = document.createComment('node1'); + node2 = rrdom.createComment('node2'); + expect(sameNodeType(node1, node2)).toBeTruthy(); + }); + + it('should return false when two nodes have different node types', () => { + let node1: Node = new Document(); + let node2: IRRNode = rrdom.createDocumentType('', '', ''); + expect(sameNodeType(node1, node2)).toBeFalsy(); + + node1 = document.implementation.createDocumentType('html', '', ''); + node2 = new RRDocument(); + expect(sameNodeType(node1, node2)).toBeFalsy(); + + node1 = document.createTextNode('node1'); + node2 = rrdom.createComment('node2'); + expect(sameNodeType(node1, node2)).toBeFalsy(); + + node1 = document.createComment('node1'); + node2 = rrdom.createTextNode('node2'); + expect(sameNodeType(node1, node2)).toBeFalsy(); + }); + }); }); From 477e47253267f09d6c739272b5158bca85d7faa2 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Thu, 12 Jan 2023 13:49:24 +1100 Subject: [PATCH 2/9] fix: selector match in iframe is case-insensitive add try catch to some fragile points --- packages/rrdom/src/diff.ts | 54 +++++++++++++-- packages/rrdom/test/diff.test.ts | 91 +++++++++++++++++++++++++ packages/rrdom/test/utils.ts | 26 +++++++ packages/rrdom/test/virtual-dom.test.ts | 24 +------ packages/rrweb/src/replay/index.ts | 19 ++++-- 5 files changed, 179 insertions(+), 35 deletions(-) create mode 100644 packages/rrdom/test/utils.ts diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 397fc10348..eff55ad881 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -97,6 +97,7 @@ export function diff( rrnodeMirror || (newTree as RRDocument).mirror || (newTree.ownerDocument as RRDocument).mirror; + // If the Mirror data has some flaws, the diff function may throw errors. We check the node consistency here to make it robust. if (!sameNodeType(oldTree, newTree)) { const calibratedOldTree = createOrGetNode( @@ -107,6 +108,20 @@ export function diff( oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); oldTree = calibratedOldTree; } + + // If the oldTree is an iframe element and its document content is automatically mounted by browsers, we need to remove them to avoid unexpected behaviors. e.g. Selector matches may be case insensitive. + if (oldTree.nodeName === 'IFRAME') { + const iframeDoc = (oldTree as HTMLIFrameElement).contentDocument; + if ( + iframeDoc && + iframeDoc.documentElement && + replayer.mirror.getId(iframeDoc.documentElement) < 0 + ) { + iframeDoc.close(); + iframeDoc.open(); + } + } + const oldChildren = oldTree.childNodes; const newChildren = newTree.childNodes; @@ -330,7 +345,11 @@ function diffChildren( // is the first old element the same as the last new element? oldStartId === newEndId ) { - parentNode.insertBefore(oldStartNode, oldEndNode.nextSibling); + try { + parentNode.insertBefore(oldStartNode, oldEndNode.nextSibling); + } catch (e) { + console.warn(e); + } diff(oldStartNode, newEndNode, replayer, rrnodeMirror); oldStartNode = oldChildren[++oldStartIndex]; newEndNode = newChildren[--newEndIndex]; @@ -339,7 +358,11 @@ function diffChildren( // is the last old element the same as the first new element? oldEndId === newStartId ) { - parentNode.insertBefore(oldEndNode, oldStartNode); + try { + parentNode.insertBefore(oldEndNode, oldStartNode); + } catch (e) { + console.warn(e); + } diff(oldEndNode, newStartNode, replayer, rrnodeMirror); oldEndNode = oldChildren[--oldEndIndex]; newStartNode = newChildren[++newStartIndex]; @@ -358,7 +381,11 @@ function diffChildren( if (indexInOld) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const nodeToMove = oldChildren[indexInOld]!; - parentNode.insertBefore(nodeToMove, oldStartNode); + try { + parentNode.insertBefore(nodeToMove, oldStartNode); + } catch (e) { + console.warn(e); + } diff(nodeToMove, newStartNode, replayer, rrnodeMirror); oldChildren[indexInOld] = undefined; } else { @@ -381,7 +408,11 @@ function diffChildren( oldChildren[oldStartIndex] = undefined; oldStartNode = undefined; } - parentNode.insertBefore(newNode, oldStartNode || null); + try { + parentNode.insertBefore(newNode, oldStartNode || null); + } catch (e) { + console.warn(e); + } diff(newNode, newStartNode, replayer, rrnodeMirror); } newStartNode = newChildren[++newStartIndex]; @@ -403,14 +434,22 @@ function diffChildren( replayer.mirror, rrnodeMirror, ); - parentNode.insertBefore(newNode, referenceNode); + try { + parentNode.insertBefore(newNode, referenceNode); + } catch (e) { + console.warn(e); + } diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror); } } else if (newStartIndex > newEndIndex) { for (; oldStartIndex <= oldEndIndex; oldStartIndex++) { const node = oldChildren[oldStartIndex]; if (node) { - parentNode.removeChild(node); + try { + parentNode.removeChild(node); + } catch (e) { + console.warn(e); + } replayer.mirror.removeNodeFromMap(node); } } @@ -469,6 +508,7 @@ export function sameNodeType(node1: Node, node2: IRRNode) { if (node1.nodeType !== node2.nodeType) return false; return ( node1.nodeType !== node1.ELEMENT_NODE || - (node1 as HTMLElement).tagName === (node2 as IRRElement).tagName + (node1 as HTMLElement).tagName.toUpperCase() === + (node2 as IRRElement).tagName ); } diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 0918335425..d696945dc3 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -1,6 +1,8 @@ /** * @jest-environment jsdom */ +import * as path from 'path'; +import * as puppeteer from 'puppeteer'; import { getDefaultSN, RRDocument, RRMediaElement } from '../src'; import { createOrGetNode, @@ -23,6 +25,7 @@ import type { styleSheetRuleData, } from '@rrweb/types'; import { EventType, IncrementalSource } from '@rrweb/types'; +import { compileTSCode } from './utils'; const elementSn = { type: RRNodeType.Element, @@ -1272,6 +1275,94 @@ describe('diff algorithm for rrdom', () => { expect(element.nodeType).toBe(element.DOCUMENT_TYPE_NODE); expect(mirror.getId(element)).toEqual(-1); }); + + it('selectors should be case-sensitive for matching in iframe dom', async () => { + /** + * If the selector match is case insensitive, it will cause some CSS style problems in the replayer. + * This test result executed in JSDom is different from that in real browser so we use puppeteer as test environment. + */ + let browser = await puppeteer.launch(); + const page = await browser.newPage(); + await page.goto('about:blank'); + + try { + const code = await compileTSCode( + path.resolve(__dirname, '../src/index.ts'), + ); + await page.evaluate(code); + + const className = 'case-sensitive'; + // To show the selector match pattern (case sensitive) in normal dom. + const caseInsensitiveInNormalDom = await page.evaluate((className) => { + document.write( + '', + ); + const htmlEl = document.documentElement; + htmlEl.className = className.toLowerCase(); + return htmlEl.matches(`.${className.toUpperCase()}`); + }, className); + expect(caseInsensitiveInNormalDom).toBeFalsy(); + + // To show the selector match pattern (case insensitive) in auto mounted iframe dom. + const caseInsensitiveInDefaultIFrameDom = await page.evaluate( + (className) => { + const iframeEl = document.querySelector('iframe'); + const htmlEl = iframeEl?.contentDocument?.documentElement; + if (htmlEl) { + htmlEl.className = className.toLowerCase(); + return htmlEl.matches(`.${className.toUpperCase()}`); + } + }, + className, + ); + expect(caseInsensitiveInDefaultIFrameDom).toBeTruthy(); + + const iframeElId = 3, + iframeDomId = 4, + htmlElId = 5; + const result = await page.evaluate(` + const iframeEl = document.querySelector('iframe'); + + // Construct a virtual dom tree. + const rrDocument = new rrdom.RRDocument(); + const rrIframeEl = rrDocument.createElement('iframe'); + rrDocument.mirror.add(rrIframeEl, rrdom.getDefaultSN(rrIframeEl, ${iframeElId})); + rrDocument.appendChild(rrIframeEl); + rrDocument.mirror.add( + rrIframeEl.contentDocument, + rrdom.getDefaultSN(rrIframeEl.contentDocument, ${iframeDomId}), + ); + const rrDocType = rrDocument.createDocumentType('html', '', ''); + rrIframeEl.contentDocument.appendChild(rrDocType); + const rrHtmlEl = rrDocument.createElement('html'); + rrDocument.mirror.add(rrHtmlEl, rrdom.getDefaultSN(rrHtmlEl, ${htmlElId})); + rrIframeEl.contentDocument.appendChild(rrHtmlEl); + + const replayer = { + mirror: rrdom.createMirror(), + applyCanvas: () => {}, + applyInput: () => {}, + applyScroll: () => {}, + applyStyleSheetMutation: () => {}, + }; + rrdom.diff(iframeEl, rrIframeEl, replayer); + + iframeEl.contentDocument.documentElement.className = + '${className.toLowerCase()}'; + iframeEl.contentDocument.childNodes.length === 2 && + replayer.mirror.getId(iframeEl.contentDocument.documentElement) === ${htmlElId} && + // To test whether the selector match of the updated iframe document is case sensitive or not. + !iframeEl.contentDocument.documentElement.matches( + '.${className.toUpperCase()}', + ); + `); + // IFrame document has two children, mirror id of documentElement is ${htmlElId}, and selectors should be case-sensitive for matching in iframe dom (consistent with the normal dom). + expect(result).toBeTruthy(); + } finally { + await page.close(); + await browser.close(); + } + }); }); describe('create or get a Node', () => { diff --git a/packages/rrdom/test/utils.ts b/packages/rrdom/test/utils.ts new file mode 100644 index 0000000000..a189ae365a --- /dev/null +++ b/packages/rrdom/test/utils.ts @@ -0,0 +1,26 @@ +import * as rollup from 'rollup'; +import * as typescript from 'rollup-plugin-typescript2'; +import resolve from '@rollup/plugin-node-resolve'; +const _typescript = (typescript as unknown) as typeof typescript.default; + +/** + * Use rollup to compile an input TS script into JS code string. + */ +export async function compileTSCode(inputFilePath: string) { + const bundle = await rollup.rollup({ + input: inputFilePath, + plugins: [ + (resolve() as unknown) as rollup.Plugin, + (_typescript({ + tsconfigOverride: { compilerOptions: { module: 'ESNext' } }, + }) as unknown) as rollup.Plugin, + ], + }); + const { + output: [{ code: _code }], + } = await bundle.generate({ + name: 'rrdom', + format: 'iife', + }); + return _code; +} diff --git a/packages/rrdom/test/virtual-dom.test.ts b/packages/rrdom/test/virtual-dom.test.ts index e414e230e8..0b25f684b2 100644 --- a/packages/rrdom/test/virtual-dom.test.ts +++ b/packages/rrdom/test/virtual-dom.test.ts @@ -4,9 +4,6 @@ import * as fs from 'fs'; import * as path from 'path'; import * as puppeteer from 'puppeteer'; -import * as rollup from 'rollup'; -import resolve from '@rollup/plugin-node-resolve'; -import * as typescript from 'rollup-plugin-typescript2'; import { JSDOM } from 'jsdom'; import { cdataNode, @@ -29,8 +26,8 @@ import { RRElement, BaseRRNode as RRNode, } from '../src'; +import { compileTSCode } from './utils'; -const _typescript = (typescript as unknown) as typeof typescript.default; const printRRDomCode = ` /** * Print the RRDom as a string. @@ -61,7 +58,7 @@ describe('RRDocument for browser environment', () => { describe('create a RRNode from a real Node', () => { it('should support quicksmode documents', () => { - // seperate jsdom document as changes to the document would otherwise bleed into other tests + // separate jsdom document as changes to the document would otherwise bleed into other tests const dom = new JSDOM(); const document = dom.window.document; @@ -219,22 +216,7 @@ describe('RRDocument for browser environment', () => { beforeAll(async () => { browser = await puppeteer.launch(); - const bundle = await rollup.rollup({ - input: path.resolve(__dirname, '../src/index.ts'), - plugins: [ - (resolve() as unknown) as rollup.Plugin, - (_typescript({ - tsconfigOverride: { compilerOptions: { module: 'ESNext' } }, - }) as unknown) as rollup.Plugin, - ], - }); - const { - output: [{ code: _code }], - } = await bundle.generate({ - name: 'rrdom', - format: 'iife', - }); - code = _code; + code = await compileTSCode(path.resolve(__dirname, '../src/index.ts')); }); afterAll(async () => { await browser.close(); diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index ce4dc6d8cb..77ca3c6bb7 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -231,13 +231,18 @@ export class Replayer { this.applyStyleDeclaration(data, styleSheet); }, }; - this.iframe.contentDocument && - diff( - this.iframe.contentDocument, - this.virtualDom, - replayerHandler, - this.virtualDom.mirror, - ); + if (this.iframe.contentDocument) + try { + diff( + this.iframe.contentDocument, + this.virtualDom, + replayerHandler, + this.virtualDom.mirror, + ); + } catch (e) { + console.warn(e); + } + this.virtualDom.destroyTree(); this.usingVirtualDom = false; From 011abf1025e1780da0a0499aa080711767c3c80b Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Thu, 12 Jan 2023 15:13:49 +1100 Subject: [PATCH 3/9] test: increase timeout value for Jest --- packages/rrdom/test/diff.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index d696945dc3..989f198571 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -1124,6 +1124,8 @@ describe('diff algorithm for rrdom', () => { }); describe('diff iframe elements', () => { + jest.setTimeout(60_000); + it('should add an element to the contentDocument of an iframe element', () => { document.write(''); const node = document.createElement('iframe'); From 5f319f1304bc885de46429024fb17a38f8664e4e Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Thu, 12 Jan 2023 15:25:48 +1100 Subject: [PATCH 4/9] improve code style --- packages/rrdom/test/diff.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 989f198571..a24e3ba97f 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -1283,7 +1283,7 @@ describe('diff algorithm for rrdom', () => { * If the selector match is case insensitive, it will cause some CSS style problems in the replayer. * This test result executed in JSDom is different from that in real browser so we use puppeteer as test environment. */ - let browser = await puppeteer.launch(); + const browser = await puppeteer.launch(); const page = await browser.newPage(); await page.goto('about:blank'); From 72bd7e6207b89fe1cf34f04c34b05b336a1f6488 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Tue, 31 Jan 2023 18:06:38 +1100 Subject: [PATCH 5/9] fix: failed to execute insertBefore on Node in the diff function this happens when ids of doctype or html element are changed in the virtual dom also improve the code quality --- packages/rrdom/src/diff.ts | 123 +++++++++++++--------- packages/rrdom/src/document.ts | 10 ++ packages/rrdom/test/diff.test.ts | 146 +++++++++++++++++++++++++-- packages/rrdom/test/document.test.ts | 2 +- 4 files changed, 221 insertions(+), 60 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index eff55ad881..963b52263c 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -87,17 +87,20 @@ export type ReplayerHandler = { ) => void; }; +/** + * Make the old tree to have the same structure and properties as the new tree with the diff algorithm. + * @param oldTree - The old tree to be modified. + * @param newTree - The new tree which the old tree will be modified to. + * @param replayer - A slimmed replayer instance including the mirror of the old tree. + * @param rrnodeMirror - The mirror of the new tree. + */ export function diff( oldTree: Node, newTree: IRRNode, replayer: ReplayerHandler, - rrnodeMirror?: Mirror, + rrnodeMirror: Mirror = (newTree as RRDocument).mirror || + (newTree.ownerDocument as RRDocument).mirror, ) { - rrnodeMirror = - rrnodeMirror || - (newTree as RRDocument).mirror || - (newTree.ownerDocument as RRDocument).mirror; - // If the Mirror data has some flaws, the diff function may throw errors. We check the node consistency here to make it robust. if (!sameNodeType(oldTree, newTree)) { const calibratedOldTree = createOrGetNode( @@ -108,6 +111,17 @@ export function diff( oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); oldTree = calibratedOldTree; } + // If the oldTree is a document node and the newTree has a different serialized Id, we need to update the nodeMirror. + else if ( + oldTree.nodeName === '#document' && + !nodeMatching(oldTree, newTree, replayer.mirror, rrnodeMirror) + ) { + const newMeta = rrnodeMirror.getMeta(newTree); + if (newMeta) { + replayer.mirror.removeNodeFromMap(oldTree); + replayer.mirror.add(oldTree, newMeta); + } + } // If the oldTree is an iframe element and its document content is automatically mounted by browsers, we need to remove them to avoid unexpected behaviors. e.g. Selector matches may be case insensitive. if (oldTree.nodeName === 'IFRAME') { @@ -310,40 +324,27 @@ 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); - - // 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 ( - oldStartId !== -1 && - // same first element? - oldStartId === newStartId + // same first node? + nodeMatching(oldStartNode, newStartNode, replayer.mirror, rrnodeMirror) ) { diff(oldStartNode, newStartNode, replayer, rrnodeMirror); oldStartNode = oldChildren[++oldStartIndex]; newStartNode = newChildren[++newStartIndex]; } else if ( - oldEndId !== -1 && - // same last element? - oldEndId === newEndId + // same last node? + nodeMatching(oldEndNode, newEndNode, replayer.mirror, rrnodeMirror) ) { diff(oldEndNode, newEndNode, replayer, rrnodeMirror); oldEndNode = oldChildren[--oldEndIndex]; newEndNode = newChildren[--newEndIndex]; } else if ( - oldStartId !== -1 && - // is the first old element the same as the last new element? - oldStartId === newEndId + // is the first old node the same as the last new node? + nodeMatching(oldStartNode, newEndNode, replayer.mirror, rrnodeMirror) ) { try { parentNode.insertBefore(oldStartNode, oldEndNode.nextSibling); @@ -354,9 +355,8 @@ function diffChildren( oldStartNode = oldChildren[++oldStartIndex]; newEndNode = newChildren[--newEndIndex]; } else if ( - oldEndId !== -1 && - // is the last old element the same as the first new element? - oldEndId === newStartId + // is the last old node the same as the first new node? + nodeMatching(oldEndNode, newStartNode, replayer.mirror, rrnodeMirror) ) { try { parentNode.insertBefore(oldEndNode, oldStartNode); @@ -395,19 +395,27 @@ function diffChildren( rrnodeMirror, ); - /** - * A mounted iframe element has an automatically created HTML element. - * We should delete it before insert a serialized one. Otherwise, an error 'Only one element on document allowed' will be thrown. - */ if ( parentNode.nodeName === '#document' && - replayer.mirror.getMeta(newNode)?.type === RRNodeType.Element && - (parentNode as Document).documentElement + oldStartNode && + /** + * Special case 1: one document isn't allowed to have two doctype nodes at the same time, so we need to remove the old one first before inserting the new one. + * How this case happens: A parent document in the old tree already has a doctype node with an id e.g. #1. A new full snapshot rebuilds the replayer with a new doctype node with another id #2. According to the algorithm, the new doctype node will be inserted before the old one, which is not allowed by the Document standard. + */ + ((newNode.nodeType === newNode.DOCUMENT_TYPE_NODE && + oldStartNode.nodeType === oldStartNode.DOCUMENT_TYPE_NODE) || + /** + * Special case 2: one document isn't allowed to have two HTMLElements at the same time, so we need to remove the old one first before inserting the new one. + * How this case happens: A mounted iframe element has an automatically created HTML element. We should delete it before inserting a serialized one. Otherwise, an error 'Only one element on document allowed' will be thrown. + */ + (newNode.nodeType === newNode.ELEMENT_NODE && + oldStartNode.nodeType === oldStartNode.ELEMENT_NODE)) ) { - parentNode.removeChild((parentNode as Document).documentElement); - oldChildren[oldStartIndex] = undefined; - oldStartNode = undefined; + parentNode.removeChild(oldStartNode); + replayer.mirror.removeNodeFromMap(oldStartNode); + oldStartNode = oldChildren[++oldStartIndex]; } + try { parentNode.insertBefore(newNode, oldStartNode || null); } catch (e) { @@ -420,14 +428,11 @@ function diffChildren( } if (oldStartIndex > oldEndIndex) { const referenceRRNode = newChildren[newEndIndex + 1]; - let referenceNode = null; + let referenceNode: Node | null = null; if (referenceRRNode) - parentNode.childNodes.forEach((child) => { - if ( - replayer.mirror.getId(child) === rrnodeMirror.getId(referenceRRNode) - ) - referenceNode = child; - }); + referenceNode = replayer.mirror.getNode( + rrnodeMirror.getId(referenceRRNode), + ); for (; newStartIndex <= newEndIndex; ++newStartIndex) { const newNode = createOrGetNode( newChildren[newStartIndex], @@ -444,13 +449,12 @@ function diffChildren( } else if (newStartIndex > newEndIndex) { for (; oldStartIndex <= oldEndIndex; oldStartIndex++) { const node = oldChildren[oldStartIndex]; - if (node) { - try { - parentNode.removeChild(node); - } catch (e) { - console.warn(e); - } + if (!node || !parentNode.contains(node)) continue; + try { + parentNode.removeChild(node); replayer.mirror.removeNodeFromMap(node); + } catch (e) { + console.warn(e); } } } @@ -512,3 +516,22 @@ export function sameNodeType(node1: Node, node2: IRRNode) { (node2 as IRRElement).tagName ); } + +/** + * To check whether two nodes are matching. If so, they are supposed to have the same serialized Id and node type. If they are both Elements, their tagNames should be the same as well. Otherwise, they are not matching. + */ +export function nodeMatching( + node1: Node, + node2: IRRNode, + domMirror: NodeMirror, + rrdomMirror: Mirror, +): boolean { + const node1Id = domMirror.getId(node1); + const node2Id = rrdomMirror.getId(node2); + // 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 (node1Id === -1 || node1Id !== node2Id) return false; + return sameNodeType(node1, node2); +} diff --git a/packages/rrdom/src/document.ts b/packages/rrdom/src/document.ts index f8ee5b8686..9f76f82123 100644 --- a/packages/rrdom/src/document.ts +++ b/packages/rrdom/src/document.ts @@ -204,6 +204,12 @@ export function BaseRRDocumentImpl< public readonly RRNodeType = RRNodeType.Document; public textContent: string | null = null; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + constructor(...args: any[]) { + super(args); + this.ownerDocument = this; + } + public get documentElement(): IRRElement | null { return ( (this.childNodes.find( @@ -258,6 +264,7 @@ export function BaseRRDocumentImpl< } childNode.parentElement = null; childNode.parentNode = this; + childNode.ownerDocument = this.ownerDocument; this.childNodes.push(childNode); return childNode; } @@ -285,6 +292,7 @@ export function BaseRRDocumentImpl< this.childNodes.splice(childIndex, 0, newChild); newChild.parentElement = null; newChild.parentNode = this; + newChild.ownerDocument = this.ownerDocument; return newChild; } @@ -522,6 +530,7 @@ export function BaseRRElementImpl< this.childNodes.push(newChild); newChild.parentNode = this; newChild.parentElement = this; + newChild.ownerDocument = this.ownerDocument; return newChild; } @@ -535,6 +544,7 @@ export function BaseRRElementImpl< this.childNodes.splice(childIndex, 0, newChild); newChild.parentElement = this; newChild.parentNode = this; + newChild.ownerDocument = this.ownerDocument; return newChild; } diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index a24e3ba97f..74adbcc0ff 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -3,19 +3,26 @@ */ import * as path from 'path'; import * as puppeteer from 'puppeteer'; -import { getDefaultSN, RRDocument, RRMediaElement } from '../src'; +import { + NodeType as RRNodeType, + serializedNodeWithId, + createMirror, + Mirror as NodeMirror, +} from 'rrweb-snapshot'; +import { + buildFromDom, + getDefaultSN, + Mirror as RRNodeMirror, + RRDocument, + RRMediaElement, +} from '../src'; import { createOrGetNode, diff, ReplayerHandler, + nodeMatching, sameNodeType, } from '../src/diff'; -import { - NodeType as RRNodeType, - serializedNodeWithId, - createMirror, - Mirror, -} from 'rrweb-snapshot'; import type { IRRElement, IRRNode } from '../src/document'; import { Replayer } from 'rrweb'; import type { @@ -26,6 +33,7 @@ import type { } from '@rrweb/types'; import { EventType, IncrementalSource } from '@rrweb/types'; import { compileTSCode } from './utils'; +import { printRRDom } from '../src/index'; const elementSn = { type: RRNodeType.Element, @@ -53,7 +61,7 @@ type RRNode = IRRNode; function createTree( treeNode: ElementType, rrDocument?: RRDocument, - mirror: Mirror = createMirror(), + mirror: NodeMirror = createMirror(), ): Node | RRNode { type TNode = typeof rrDocument extends RRDocument ? RRNode : Node; let root: TNode; @@ -95,7 +103,7 @@ function shuffle(list: number[]) { } describe('diff algorithm for rrdom', () => { - let mirror: Mirror; + let mirror: NodeMirror; let replayer: ReplayerHandler; beforeEach(() => { @@ -1278,6 +1286,65 @@ describe('diff algorithm for rrdom', () => { expect(mirror.getId(element)).toEqual(-1); }); + it('should remove children from document before adding new nodes 4', () => { + /** + * This case aims to test whether the diff function can remove all the old doctype and html element from the document before adding new doctype and html element. + * If not, the diff function will throw errors or warnings. + */ + // Mock the original console.warn function to make the test fail once console.warn is called. + const warn = jest.spyOn(global.console, 'warn'); + + document.write(''); + const rrdom = new RRDocument(); + /** + * Make the structure of document and RRDom look like this: + * -2 Document + * -3 DocumentType + * -4 HTML + * -5 HEAD + * -6 BODY + */ + buildFromDom(document, mirror, rrdom); + expect(mirror.getId(document)).toBe(-2); + expect(mirror.getId(document.body)).toBe(-6); + expect(rrdom.mirror.getId(rrdom)).toBe(-2); + expect(rrdom.mirror.getId(rrdom.body)).toBe(-6); + + rrdom.childNodes = []; + /** + * Rebuild the rrdom and make it looks like this: + * -7 RRDocument + * -8 RRDocumentType + * -9 HTML + * -10 HEAD + * -11 BODY + */ + buildFromDom(document, undefined, rrdom); + // Keep the ids of real document unchanged. + expect(mirror.getId(document)).toBe(-2); + expect(mirror.getId(document.body)).toBe(-6); + + expect(rrdom.mirror.getId(rrdom)).toBe(-7); + expect(rrdom.mirror.getId(rrdom.body)).toBe(-11); + + // Diff the document with the new rrdom. + diff(document, rrdom, replayer); + // Check that warn was not called (fail on warning) + expect(warn).not.toHaveBeenCalled(); + + // Check that the old nodes are removed from the NodeMirror. + [-2, -3, -4, -5, -6].forEach((id) => + expect(mirror.getNode(id)).toBeNull(), + ); + expect(mirror.getId(document)).toBe(-7); + expect(mirror.getId(document.doctype)).toBe(-8); + expect(mirror.getId(document.documentElement)).toBe(-9); + expect(mirror.getId(document.head)).toBe(-10); + expect(mirror.getId(document.body)).toBe(-11); + + warn.mockRestore(); + }); + it('selectors should be case-sensitive for matching in iframe dom', async () => { /** * If the selector match is case insensitive, it will cause some CSS style problems in the replayer. @@ -1656,4 +1723,65 @@ describe('diff algorithm for rrdom', () => { expect(sameNodeType(node1, node2)).toBeFalsy(); }); }); + + describe('test nodeMatching function', () => { + const rrdom = new RRDocument(); + const NodeMirror = createMirror(); + const rrdomMirror = new RRNodeMirror(); + beforeEach(() => { + NodeMirror.reset(); + rrdomMirror.reset(); + }); + + it('should return false when two nodes have different Ids', () => { + const node1 = document.createElement('div'); + const node2 = rrdom.createElement('div'); + NodeMirror.add(node1, getDefaultSN(node2, 1)); + rrdomMirror.add(node2, getDefaultSN(node2, 2)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + }); + + it('should return false when two nodes have same Ids but different node types', () => { + // Compare an element with a comment node + let node1: Node = document.createElement('div'); + NodeMirror.add(node1, getDefaultSN(rrdom.createElement('div'), 1)); + let node2: IRRNode = rrdom.createComment('test'); + rrdomMirror.add(node2, getDefaultSN(node2, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + + // Compare an element node with a text node + node2 = rrdom.createTextNode(''); + rrdomMirror.add(node2, getDefaultSN(node2, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + + // Compare a document with a text node + node1 = new Document(); + NodeMirror.add(node1, getDefaultSN(rrdom, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + + // Compare a document with a document type node + node2 = rrdom.createDocumentType('', '', ''); + rrdomMirror.add(node2, getDefaultSN(node2, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + }); + + it('should compare two elements', () => { + // Compare two elements with different tagNames + let node1 = document.createElement('div'); + let node2 = rrdom.createElement('span'); + NodeMirror.add(node1, getDefaultSN(rrdom.createElement('div'), 1)); + rrdomMirror.add(node2, getDefaultSN(node2, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + + // Compare two elements with same tagNames but different attributes + node2 = rrdom.createElement('div'); + node2.setAttribute('class', 'test'); + rrdomMirror.add(node2, getDefaultSN(node2, 1)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeTruthy(); + + // Should return false when two elements have same tagNames and attributes but different children + rrdomMirror.add(node2, getDefaultSN(node2, 2)); + expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy(); + }); + }); }); diff --git a/packages/rrdom/test/document.test.ts b/packages/rrdom/test/document.test.ts index c9ee13c3a0..47d9fa73e8 100644 --- a/packages/rrdom/test/document.test.ts +++ b/packages/rrdom/test/document.test.ts @@ -133,7 +133,7 @@ describe('Basic RRDocument implementation', () => { expect(node.parentElement).toEqual(null); expect(node.childNodes).toBeInstanceOf(Array); expect(node.childNodes.length).toBe(0); - expect(node.ownerDocument).toBeUndefined(); + expect(node.ownerDocument).toBe(node); expect(node.textContent).toBeNull(); expect(node.RRNodeType).toBe(RRNodeType.Document); expect(node.nodeType).toBe(document.nodeType); From 1dd69d815250067ec4b36eb33c3de768437654c9 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Wed, 1 Feb 2023 13:24:34 +1100 Subject: [PATCH 6/9] refactor diff function to make the code cleaner --- packages/rrdom/src/diff.ts | 157 ++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 88 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 963b52263c..09ece174d8 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -10,7 +10,6 @@ import type { import type { IRRCDATASection, IRRComment, - IRRDocument, IRRDocumentType, IRRElement, IRRNode, @@ -111,50 +110,26 @@ export function diff( oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); oldTree = calibratedOldTree; } - // If the oldTree is a document node and the newTree has a different serialized Id, we need to update the nodeMirror. - else if ( - oldTree.nodeName === '#document' && - !nodeMatching(oldTree, newTree, replayer.mirror, rrnodeMirror) - ) { - const newMeta = rrnodeMirror.getMeta(newTree); - if (newMeta) { - replayer.mirror.removeNodeFromMap(oldTree); - replayer.mirror.add(oldTree, newMeta); - } - } - - // If the oldTree is an iframe element and its document content is automatically mounted by browsers, we need to remove them to avoid unexpected behaviors. e.g. Selector matches may be case insensitive. - if (oldTree.nodeName === 'IFRAME') { - const iframeDoc = (oldTree as HTMLIFrameElement).contentDocument; - if ( - iframeDoc && - iframeDoc.documentElement && - replayer.mirror.getId(iframeDoc.documentElement) < 0 - ) { - iframeDoc.close(); - iframeDoc.open(); - } - } - - const oldChildren = oldTree.childNodes; - const newChildren = newTree.childNodes; - - if (oldChildren.length > 0 || newChildren.length > 0) { - diffChildren( - Array.from(oldChildren), - newChildren, - oldTree, - replayer, - rrnodeMirror, - ); - } let inputDataToApply = null, scrollDataToApply = null; switch (newTree.RRNodeType) { case RRNodeType.Document: { - const newRRDocument = newTree as IRRDocument; - scrollDataToApply = (newRRDocument as RRDocument).scrollData; + scrollDataToApply = (newTree as RRDocument).scrollData; + /** + * Special cases for updating the document node: + * Case 1: If the oldTree is the content document of an iframe element and its content (HTML, HEAD, and BODY) is automatically mounted by browsers, we need to remove them to avoid unexpected behaviors. e.g. Selector matches may be case insensitive. + * Case 2: The newTree has a different serialized Id (a different document object), we need to reopen it and update the nodeMirror. + */ + if (!nodeMatching(oldTree, newTree, replayer.mirror, rrnodeMirror)) { + const newMeta = rrnodeMirror.getMeta(newTree); + if (newMeta) { + replayer.mirror.removeNodeFromMap(oldTree); + (oldTree as Document).close(); + (oldTree as Document).open(); + replayer.mirror.add(oldTree, newMeta); + } + } break; } case RRNodeType.Element: { @@ -182,38 +157,50 @@ export function diff( oldMediaElement.playbackRate = newMediaRRElement.playbackRate; break; } - case 'CANVAS': - { - const rrCanvasElement = newTree as RRCanvasElement; - // This canvas element is created with initial data in an iframe element. https://github.com/rrweb-io/rrweb/pull/944 - if (rrCanvasElement.rr_dataURL !== null) { - const image = document.createElement('img'); - image.onload = () => { - const ctx = (oldElement as HTMLCanvasElement).getContext('2d'); - if (ctx) { - ctx.drawImage(image, 0, 0, image.width, image.height); - } - }; - image.src = rrCanvasElement.rr_dataURL; - } - rrCanvasElement.canvasMutations.forEach((canvasMutation) => - replayer.applyCanvas( - canvasMutation.event, - canvasMutation.mutation, - oldTree as HTMLCanvasElement, - ), - ); + case 'CANVAS': { + const rrCanvasElement = newTree as RRCanvasElement; + // This canvas element is created with initial data in an iframe element. https://github.com/rrweb-io/rrweb/pull/944 + if (rrCanvasElement.rr_dataURL !== null) { + const image = document.createElement('img'); + image.onload = () => { + const ctx = (oldElement as HTMLCanvasElement).getContext('2d'); + if (ctx) { + ctx.drawImage(image, 0, 0, image.width, image.height); + } + }; + image.src = rrCanvasElement.rr_dataURL; } + rrCanvasElement.canvasMutations.forEach((canvasMutation) => + replayer.applyCanvas( + canvasMutation.event, + canvasMutation.mutation, + oldTree as HTMLCanvasElement, + ), + ); break; - case 'STYLE': - { - const styleSheet = (oldElement as HTMLStyleElement).sheet; - styleSheet && - (newTree as RRStyleElement).rules.forEach((data) => - replayer.applyStyleSheetMutation(data, styleSheet), - ); - } + } + case 'STYLE': { + const styleSheet = (oldElement as HTMLStyleElement).sheet; + styleSheet && + (newTree as RRStyleElement).rules.forEach((data) => + replayer.applyStyleSheetMutation(data, styleSheet), + ); break; + } + case 'IFRAME': { + const oldContentDocument = (oldTree as HTMLIFrameElement) + .contentDocument; + // If the iframe is cross-origin, the contentDocument will be null. + if (!oldContentDocument) break; + // IFrame element doesn't have child nodes, so here we update its content document separately. + diff( + oldContentDocument, + (newTree as RRIFrameElement).contentDocument, + replayer, + rrnodeMirror, + ); + break; + } } if (newRRElement.shadowRoot) { if (!oldElement.shadowRoot) oldElement.attachShadow({ mode: 'open' }); @@ -247,31 +234,25 @@ export function diff( default: } + const oldChildren = oldTree.childNodes; + const newChildren = newTree.childNodes; + + if (oldChildren.length > 0 || newChildren.length > 0) { + diffChildren( + Array.from(oldChildren), + newChildren, + oldTree, + replayer, + rrnodeMirror, + ); + } + scrollDataToApply && replayer.applyScroll(scrollDataToApply, true); /** * Input data need to get applied after all children of this node are updated. * Otherwise when we set a value for a select element whose options are empty, the value won't actually update. */ inputDataToApply && replayer.applyInput(inputDataToApply); - - // IFrame element doesn't have child nodes. - if (newTree.nodeName === 'IFRAME') { - const oldContentDocument = (oldTree as HTMLIFrameElement).contentDocument; - const newIFrameElement = newTree as RRIFrameElement; - // If the iframe is cross-origin, the contentDocument will be null. - if (oldContentDocument) { - const sn = rrnodeMirror.getMeta(newIFrameElement.contentDocument); - if (sn) { - replayer.mirror.add(oldContentDocument, { ...sn }); - } - diff( - oldContentDocument, - newIFrameElement.contentDocument, - replayer, - rrnodeMirror, - ); - } - } } function diffProps( From fb6db8cc254d2c0ff05d918297fa4be00c058ff6 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Wed, 1 Feb 2023 16:31:26 +1100 Subject: [PATCH 7/9] fix: virtual nodes are passed to plugin's onBuild function --- packages/rrdom/src/diff.ts | 21 +++++++++++++++------ packages/rrdom/test/diff.test.ts | 10 +++++++++- packages/rrweb/src/replay/index.ts | 9 +++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 09ece174d8..e78c290119 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -84,6 +84,8 @@ export type ReplayerHandler = { data: styleDeclarationData | styleSheetRuleData, styleSheet: CSSStyleSheet, ) => void; + // Similar to the `afterAppend` callback in the `rrweb-snapshot` package. It's a postorder traversal of the newly appended nodes. + afterAppend?(node: Node, id: number): void; }; /** @@ -109,6 +111,7 @@ export function diff( ); oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); oldTree = calibratedOldTree; + replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); } let inputDataToApply = null, @@ -128,6 +131,7 @@ export function diff( (oldTree as Document).close(); (oldTree as Document).open(); replayer.mirror.add(oldTree, newMeta); + replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); } } break; @@ -303,7 +307,7 @@ function diffChildren( newStartNode = newChildren[newStartIndex], newEndNode = newChildren[newEndIndex]; let oldIdToIndex: Record | undefined = undefined, - indexInOld; + indexInOld: number | undefined = undefined; while (oldStartIndex <= oldEndIndex && newStartIndex <= newEndIndex) { if (oldStartNode === undefined) { oldStartNode = oldChildren[++oldStartIndex]; @@ -359,9 +363,12 @@ function diffChildren( } } indexInOld = oldIdToIndex[rrnodeMirror.getId(newStartNode)]; - if (indexInOld) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const nodeToMove = oldChildren[indexInOld]!; + const nodeToMove = oldChildren[indexInOld]; + if ( + indexInOld !== undefined && + nodeToMove && + nodeMatching(nodeToMove, newStartNode, replayer.mirror, rrnodeMirror) + ) { try { parentNode.insertBefore(nodeToMove, oldStartNode); } catch (e) { @@ -399,10 +406,11 @@ function diffChildren( try { parentNode.insertBefore(newNode, oldStartNode || null); + diff(newNode, newStartNode, replayer, rrnodeMirror); + replayer.afterAppend?.(newNode, replayer.mirror.getId(newNode)); } catch (e) { console.warn(e); } - diff(newNode, newStartNode, replayer, rrnodeMirror); } newStartNode = newChildren[++newStartIndex]; } @@ -422,10 +430,11 @@ function diffChildren( ); try { parentNode.insertBefore(newNode, referenceNode); + diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror); + replayer.afterAppend?.(newNode, replayer.mirror.getId(newNode)); } catch (e) { console.warn(e); } - diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror); } } else if (newStartIndex > newEndIndex) { for (; oldStartIndex <= oldEndIndex; oldStartIndex++) { diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 74adbcc0ff..62fb5bb1bd 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -1060,7 +1060,15 @@ describe('diff algorithm for rrdom', () => { } as serializedNodeWithId; mirror.add(unreliableChild, unreliableSN); parentNode.appendChild(unreliableChild); - mirror.add(document.createElement('div'), { ...unreliableSN, id: 2 }); + createTree( + { + tagName: 'div', + id: 2, + children: [], + }, + undefined, + mirror, + ); const rrParentNode = createTree( { diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index 77ca3c6bb7..465df88a7d 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -230,6 +230,11 @@ export class Replayer { else if (data.source === IncrementalSource.StyleDeclaration) this.applyStyleDeclaration(data, styleSheet); }, + afterAppend: (node: Node, id: number) => { + for (const plugin of this.config.plugins || []) { + if (plugin.onBuild) plugin.onBuild(node, { id, replayer: this }); + } + }, }; if (this.iframe.contentDocument) try { @@ -863,6 +868,8 @@ export class Replayer { ); } + // Skip the plugin onBuild callback in the virtual dom mode + if (this.usingVirtualDom) return; for (const plugin of this.config.plugins || []) { if (plugin.onBuild) plugin.onBuild(builtNode, { @@ -1480,6 +1487,8 @@ export class Replayer { return; } const afterAppend = (node: Node | RRNode, id: number) => { + // Skip the plugin onBuild callback for virtual dom + if (this.usingVirtualDom) return; for (const plugin of this.config.plugins || []) { if (plugin.onBuild) plugin.onBuild(node, { id, replayer: this }); } From fee57d874f84ebedd9d9a70a39640cec2e8aba9a Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Wed, 1 Feb 2023 17:08:04 +1100 Subject: [PATCH 8/9] refactor the diff function and adjust the order of diff work. --- packages/rrdom/src/diff.ts | 146 +++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 56 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index e78c290119..da094180c1 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -114,11 +114,34 @@ export function diff( replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); } - let inputDataToApply = null, - scrollDataToApply = null; + diffBeforeUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror); + + const oldChildren = oldTree.childNodes; + const newChildren = newTree.childNodes; + if (oldChildren.length > 0 || newChildren.length > 0) { + diffChildren( + Array.from(oldChildren), + newChildren, + oldTree, + replayer, + rrnodeMirror, + ); + } + + diffAfterUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror); +} + +/** + * Do some preparation work before updating the children of the old tree. + */ +function diffBeforeUpdatingChildren( + oldTree: Node, + newTree: IRRNode, + replayer: ReplayerHandler, + rrnodeMirror: Mirror, +) { switch (newTree.RRNodeType) { case RRNodeType.Document: { - scrollDataToApply = (newTree as RRDocument).scrollData; /** * Special cases for updating the document node: * Case 1: If the oldTree is the content document of an iframe element and its content (HTML, HEAD, and BODY) is automatically mounted by browsers, we need to remove them to avoid unexpected behaviors. e.g. Selector matches may be case insensitive. @@ -139,9 +162,68 @@ export function diff( case RRNodeType.Element: { const oldElement = oldTree as HTMLElement; const newRRElement = newTree as IRRElement; + switch (newRRElement.tagName) { + case 'IFRAME': { + const oldContentDocument = (oldTree as HTMLIFrameElement) + .contentDocument; + // If the iframe is cross-origin, the contentDocument will be null. + if (!oldContentDocument) break; + // IFrame element doesn't have child nodes, so here we update its content document separately. + diff( + oldContentDocument, + (newTree as RRIFrameElement).contentDocument, + replayer, + rrnodeMirror, + ); + break; + } + } + if (newRRElement.shadowRoot) { + if (!oldElement.shadowRoot) oldElement.attachShadow({ mode: 'open' }); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const oldChildren = oldElement.shadowRoot!.childNodes; + const newChildren = newRRElement.shadowRoot.childNodes; + if (oldChildren.length > 0 || newChildren.length > 0) + diffChildren( + Array.from(oldChildren), + newChildren, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + oldElement.shadowRoot!, + replayer, + rrnodeMirror, + ); + } + break; + } + } +} + +/** + * Finish the diff work after updating the children of the old tree. + */ +function diffAfterUpdatingChildren( + oldTree: Node, + newTree: IRRNode, + replayer: ReplayerHandler, + rrnodeMirror: Mirror, +) { + switch (newTree.RRNodeType) { + case RRNodeType.Document: { + const scrollData = (newTree as RRDocument).scrollData; + scrollData && replayer.applyScroll(scrollData, true); + break; + } + case RRNodeType.Element: { + const oldElement = oldTree as HTMLElement; + const newRRElement = newTree as RRElement; diffProps(oldElement, newRRElement, rrnodeMirror); - scrollDataToApply = (newRRElement as RRElement).scrollData; - inputDataToApply = (newRRElement as RRElement).inputData; + newRRElement.scrollData && + replayer.applyScroll(newRRElement.scrollData, true); + /** + * Input data need to get applied after all children of this node are updated. + * Otherwise when we set a value for a select element whose options are empty, the value won't actually update. + */ + newRRElement.inputData && replayer.applyInput(newRRElement.inputData); switch (newRRElement.tagName) { case 'AUDIO': case 'VIDEO': { @@ -183,6 +265,7 @@ export function diff( ); break; } + // Props of style elements have to be updated after all children are updated. Otherwise the props can be overwritten by textContent. case 'STYLE': { const styleSheet = (oldElement as HTMLStyleElement).sheet; styleSheet && @@ -191,41 +274,12 @@ export function diff( ); break; } - case 'IFRAME': { - const oldContentDocument = (oldTree as HTMLIFrameElement) - .contentDocument; - // If the iframe is cross-origin, the contentDocument will be null. - if (!oldContentDocument) break; - // IFrame element doesn't have child nodes, so here we update its content document separately. - diff( - oldContentDocument, - (newTree as RRIFrameElement).contentDocument, - replayer, - rrnodeMirror, - ); - break; - } - } - if (newRRElement.shadowRoot) { - if (!oldElement.shadowRoot) oldElement.attachShadow({ mode: 'open' }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const oldChildren = oldElement.shadowRoot!.childNodes; - const newChildren = newRRElement.shadowRoot.childNodes; - if (oldChildren.length > 0 || newChildren.length > 0) - diffChildren( - Array.from(oldChildren), - newChildren, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - oldElement.shadowRoot!, - replayer, - rrnodeMirror, - ); } break; } case RRNodeType.Text: case RRNodeType.Comment: - case RRNodeType.CDATA: + case RRNodeType.CDATA: { if ( oldTree.textContent !== (newTree as IRRText | IRRComment | IRRCDATASection).data @@ -235,28 +289,8 @@ export function diff( | IRRComment | IRRCDATASection).data; break; - default: - } - - const oldChildren = oldTree.childNodes; - const newChildren = newTree.childNodes; - - if (oldChildren.length > 0 || newChildren.length > 0) { - diffChildren( - Array.from(oldChildren), - newChildren, - oldTree, - replayer, - rrnodeMirror, - ); + } } - - scrollDataToApply && replayer.applyScroll(scrollDataToApply, true); - /** - * Input data need to get applied after all children of this node are updated. - * Otherwise when we set a value for a select element whose options are empty, the value won't actually update. - */ - inputDataToApply && replayer.applyInput(inputDataToApply); } function diffProps( From 8eaf2d36d8e50d63a38055bb7ccd18ee298937ff Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Wed, 1 Feb 2023 22:09:45 +1100 Subject: [PATCH 9/9] call afterAppend hook in a consistent traversal order --- packages/rrdom/src/diff.ts | 62 +++++++---- packages/rrdom/test/diff.test.ts | 177 ++++++++++++++++++++++++++++++- 2 files changed, 217 insertions(+), 22 deletions(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index da094180c1..842bfb8953 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -1,4 +1,8 @@ -import { NodeType as RRNodeType, Mirror as NodeMirror } from 'rrweb-snapshot'; +import { + NodeType as RRNodeType, + Mirror as NodeMirror, + elementNode, +} from 'rrweb-snapshot'; import type { canvasMutationData, canvasEventWithTime, @@ -88,6 +92,9 @@ export type ReplayerHandler = { afterAppend?(node: Node, id: number): void; }; +// A set contains newly appended nodes. It's used to make sure the afterAppend callback can iterate newly appended nodes in the same traversal order as that in the `rrweb-snapshot` package. +let createdNodeSet: WeakSet | null = null; + /** * Make the old tree to have the same structure and properties as the new tree with the diff algorithm. * @param oldTree - The old tree to be modified. @@ -102,19 +109,12 @@ export function diff( rrnodeMirror: Mirror = (newTree as RRDocument).mirror || (newTree.ownerDocument as RRDocument).mirror, ) { - // If the Mirror data has some flaws, the diff function may throw errors. We check the node consistency here to make it robust. - if (!sameNodeType(oldTree, newTree)) { - const calibratedOldTree = createOrGetNode( - newTree, - replayer.mirror, - rrnodeMirror, - ); - oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); - oldTree = calibratedOldTree; - replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); - } - - diffBeforeUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror); + oldTree = diffBeforeUpdatingChildren( + oldTree, + newTree, + replayer, + rrnodeMirror, + ); const oldChildren = oldTree.childNodes; const newChildren = newTree.childNodes; @@ -140,6 +140,22 @@ function diffBeforeUpdatingChildren( replayer: ReplayerHandler, rrnodeMirror: Mirror, ) { + if (replayer.afterAppend && !createdNodeSet) { + createdNodeSet = new WeakSet(); + setTimeout(() => { + createdNodeSet = null; + }, 0); + } + // If the Mirror data has some flaws, the diff function may throw errors. We check the node consistency here to make it robust. + if (!sameNodeType(oldTree, newTree)) { + const calibratedOldTree = createOrGetNode( + newTree, + replayer.mirror, + rrnodeMirror, + ); + oldTree.parentNode?.replaceChild(calibratedOldTree, oldTree); + oldTree = calibratedOldTree; + } switch (newTree.RRNodeType) { case RRNodeType.Document: { /** @@ -154,7 +170,7 @@ function diffBeforeUpdatingChildren( (oldTree as Document).close(); (oldTree as Document).open(); replayer.mirror.add(oldTree, newMeta); - replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); + createdNodeSet?.add(oldTree); } } break; @@ -196,6 +212,7 @@ function diffBeforeUpdatingChildren( break; } } + return oldTree; } /** @@ -291,6 +308,10 @@ function diffAfterUpdatingChildren( break; } } + if (createdNodeSet?.has(oldTree)) { + createdNodeSet.delete(oldTree); + replayer.afterAppend?.(oldTree, replayer.mirror.getId(oldTree)); + } } function diffProps( @@ -303,8 +324,8 @@ function diffProps( for (const name in newAttributes) { const newValue = newAttributes[name]; - const sn = rrnodeMirror.getMeta(newTree); - if (sn && 'isSVG' in sn && sn.isSVG && NAMESPACES[name]) + const sn = rrnodeMirror.getMeta(newTree) as elementNode | null; + if (sn?.isSVG && NAMESPACES[name]) oldTree.setAttributeNS(NAMESPACES[name], name, newValue); else if (newTree.tagName === 'CANVAS' && name === 'rr_dataURL') { const image = document.createElement('img'); @@ -441,7 +462,6 @@ function diffChildren( try { parentNode.insertBefore(newNode, oldStartNode || null); diff(newNode, newStartNode, replayer, rrnodeMirror); - replayer.afterAppend?.(newNode, replayer.mirror.getId(newNode)); } catch (e) { console.warn(e); } @@ -465,7 +485,6 @@ function diffChildren( try { parentNode.insertBefore(newNode, referenceNode); diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror); - replayer.afterAppend?.(newNode, replayer.mirror.getId(newNode)); } catch (e) { console.warn(e); } @@ -526,6 +545,11 @@ export function createOrGetNode( } if (sn) domMirror.add(node, { ...sn }); + try { + createdNodeSet?.add(node); + } catch (e) { + // Just for safety concern. + } return node; } diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 62fb5bb1bd..536641b075 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -33,7 +33,6 @@ import type { } from '@rrweb/types'; import { EventType, IncrementalSource } from '@rrweb/types'; import { compileTSCode } from './utils'; -import { printRRDom } from '../src/index'; const elementSn = { type: RRNodeType.Element, @@ -114,7 +113,9 @@ describe('diff algorithm for rrdom', () => { applyInput: () => {}, applyScroll: () => {}, applyStyleSheetMutation: () => {}, + afterAppend: () => {}, }; + document.write(''); }); describe('diff single node', () => { @@ -133,7 +134,7 @@ describe('diff algorithm for rrdom', () => { x: 0, y: 0, }; - replayer.applyScroll = jest.fn(); + const applyScrollFn = jest.spyOn(replayer, 'applyScroll'); diff(document, rrNode, replayer); expect(document.childNodes.length).toEqual(1); expect(document.childNodes[0]).toBeInstanceOf(DocumentType); @@ -142,7 +143,24 @@ describe('diff algorithm for rrdom', () => { '-//W3C//DTD XHTML 1.0 Transitional//EN', ); expect(document.doctype?.systemId).toEqual(''); - expect(replayer.applyScroll).toBeCalledTimes(1); + expect(applyScrollFn).toHaveBeenCalledTimes(1); + applyScrollFn.mockRestore(); + }); + + it('should apply scroll data on an element', () => { + const element = document.createElement('div'); + const rrDocument = new RRDocument(); + const rrNode = rrDocument.createElement('div'); + rrNode.scrollData = { + source: IncrementalSource.Scroll, + id: 0, + x: 0, + y: 0, + }; + const applyScrollFn = jest.spyOn(replayer, 'applyScroll'); + diff(element, rrNode, replayer); + expect(applyScrollFn).toHaveBeenCalledTimes(1); + applyScrollFn.mockRestore(); }); it('should apply input data on an input element', () => { @@ -1442,6 +1460,159 @@ describe('diff algorithm for rrdom', () => { }); }); + describe('afterAppend callback', () => { + it('should call afterAppend callback', () => { + const afterAppendFn = jest.spyOn(replayer, 'afterAppend'); + const node = createTree( + { + tagName: 'div', + id: 1, + }, + undefined, + mirror, + ) as Node; + + const rrdom = new RRDocument(); + const rrNode = createTree( + { + tagName: 'div', + id: 1, + children: [ + { + tagName: 'span', + id: 2, + }, + ], + }, + rrdom, + ) as RRNode; + diff(node, rrNode, replayer); + expect(afterAppendFn).toHaveBeenCalledTimes(1); + expect(afterAppendFn).toHaveBeenCalledWith(node.childNodes[0], 2); + afterAppendFn.mockRestore(); + }); + + it('should diff without afterAppend callback', () => { + replayer.afterAppend = undefined; + const rrdom = buildFromDom(document); + document.open(); + diff(document, rrdom, replayer); + replayer.afterAppend = () => {}; + }); + + it('should call afterAppend callback in the post traversal order', () => { + const afterAppendFn = jest.spyOn(replayer, 'afterAppend'); + document.open(); + + const rrdom = new RRDocument(); + rrdom.mirror.add(rrdom, getDefaultSN(rrdom, 1)); + const rrNode = createTree( + { + tagName: 'html', + id: 1, + children: [ + { + tagName: 'head', + id: 2, + }, + { + tagName: 'body', + id: 3, + children: [ + { + tagName: 'span', + id: 4, + children: [ + { + tagName: 'li', + id: 5, + }, + { + tagName: 'li', + id: 6, + }, + ], + }, + { + tagName: 'p', + id: 7, + }, + { + tagName: 'p', + id: 8, + children: [ + { + tagName: 'li', + id: 9, + }, + { + tagName: 'li', + id: 10, + }, + ], + }, + ], + }, + ], + }, + rrdom, + ) as RRNode; + diff(document, rrNode, replayer); + + expect(afterAppendFn).toHaveBeenCalledTimes(10); + // the correct traversal order + [2, 5, 6, 4, 7, 9, 10, 8, 3, 1].forEach((id, index) => { + expect((mirror.getNode(id) as HTMLElement).tagName).toEqual( + (rrdom.mirror.getNode(id) as IRRElement).tagName, + ); + expect(afterAppendFn).toHaveBeenNthCalledWith( + index + 1, + mirror.getNode(id), + id, + ); + }); + }); + + it('should only call afterAppend for newly created nodes', () => { + const afterAppendFn = jest.spyOn(replayer, 'afterAppend'); + const rrdom = buildFromDom(document, replayer.mirror) as RRDocument; + + // Append 3 nodes to rrdom. + const rrNode = createTree( + { + tagName: 'span', + id: 1, + children: [ + { + tagName: 'li', + id: 2, + }, + { + tagName: 'li', + id: 3, + }, + ], + }, + rrdom, + ) as RRNode; + rrdom.body?.appendChild(rrNode); + diff(document, rrdom, replayer); + expect(afterAppendFn).toHaveBeenCalledTimes(3); + // Should only call afterAppend for 3 newly appended nodes. + [2, 3, 1].forEach((id, index) => { + expect((mirror.getNode(id) as HTMLElement).tagName).toEqual( + (rrdom.mirror.getNode(id) as IRRElement).tagName, + ); + expect(afterAppendFn).toHaveBeenNthCalledWith( + index + 1, + mirror.getNode(id), + id, + ); + }); + afterAppendFn.mockClear(); + }); + }); + describe('create or get a Node', () => { it('create a real HTML element from RRElement', () => { const rrDocument = new RRDocument();