diff --git a/.changeset/cold-eyes-hunt.md b/.changeset/cold-eyes-hunt.md new file mode 100644 index 0000000000..ac3a8be10d --- /dev/null +++ b/.changeset/cold-eyes-hunt.md @@ -0,0 +1,8 @@ +--- +'rrdom': patch +--- + +Fix: rrdom bugs + +1. Fix a bug in the diff algorithm. +2. Omit the 'srcdoc' attribute of iframes to avoid overwriting content. diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 78628988e0..f37f298106 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -116,17 +116,7 @@ export function diff( rrnodeMirror, ); - const oldChildren = oldTree.childNodes; - const newChildren = newTree.childNodes; - if (oldChildren.length > 0 || newChildren.length > 0) { - diffChildren( - Array.from(oldChildren), - newChildren, - oldTree, - replayer, - rrnodeMirror, - ); - } + diffChildren(oldTree, newTree, replayer, rrnodeMirror); diffAfterUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror); } @@ -196,18 +186,13 @@ function diffBeforeUpdatingChildren( } 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, - ); + diffChildren( + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + oldElement.shadowRoot!, + newRRElement.shadowRoot, + replayer, + rrnodeMirror, + ); } break; } @@ -335,7 +320,8 @@ function diffProps( ctx.drawImage(image, 0, 0, image.width, image.height); } }; - } else oldTree.setAttribute(name, newValue); + } else if (newTree.tagName === 'IFRAME' && name === 'srcdoc') continue; + else oldTree.setAttribute(name, newValue); } for (const { name } of Array.from(oldAttributes)) @@ -346,12 +332,14 @@ function diffProps( } function diffChildren( - oldChildren: (Node | undefined)[], - newChildren: IRRNode[], - parentNode: Node, + oldTree: Node, + newTree: IRRNode, replayer: ReplayerHandler, rrnodeMirror: Mirror, ) { + const oldChildren: (Node | undefined)[] = Array.from(oldTree.childNodes); + const newChildren = newTree.childNodes; + if (oldChildren.length === 0 && newChildren.length === 0) return; let oldStartIndex = 0, oldEndIndex = oldChildren.length - 1, newStartIndex = 0, @@ -371,14 +359,12 @@ function diffChildren( // same first node? nodeMatching(oldStartNode, newStartNode, replayer.mirror, rrnodeMirror) ) { - diff(oldStartNode, newStartNode, replayer, rrnodeMirror); oldStartNode = oldChildren[++oldStartIndex]; newStartNode = newChildren[++newStartIndex]; } else if ( // same last node? nodeMatching(oldEndNode, newEndNode, replayer.mirror, rrnodeMirror) ) { - diff(oldEndNode, newEndNode, replayer, rrnodeMirror); oldEndNode = oldChildren[--oldEndIndex]; newEndNode = newChildren[--newEndIndex]; } else if ( @@ -386,11 +372,10 @@ function diffChildren( nodeMatching(oldStartNode, newEndNode, replayer.mirror, rrnodeMirror) ) { try { - parentNode.insertBefore(oldStartNode, oldEndNode.nextSibling); + oldTree.insertBefore(oldStartNode, oldEndNode.nextSibling); } catch (e) { console.warn(e); } - diff(oldStartNode, newEndNode, replayer, rrnodeMirror); oldStartNode = oldChildren[++oldStartIndex]; newEndNode = newChildren[--newEndIndex]; } else if ( @@ -398,11 +383,10 @@ function diffChildren( nodeMatching(oldEndNode, newStartNode, replayer.mirror, rrnodeMirror) ) { try { - parentNode.insertBefore(oldEndNode, oldStartNode); + oldTree.insertBefore(oldEndNode, oldStartNode); } catch (e) { console.warn(e); } - diff(oldEndNode, newStartNode, replayer, rrnodeMirror); oldEndNode = oldChildren[--oldEndIndex]; newStartNode = newChildren[++newStartIndex]; } else { @@ -424,11 +408,10 @@ function diffChildren( nodeMatching(nodeToMove, newStartNode, replayer.mirror, rrnodeMirror) ) { try { - parentNode.insertBefore(nodeToMove, oldStartNode); + oldTree.insertBefore(nodeToMove, oldStartNode); } catch (e) { console.warn(e); } - diff(nodeToMove, newStartNode, replayer, rrnodeMirror); oldChildren[indexInOld] = undefined; } else { const newNode = createOrGetNode( @@ -438,7 +421,7 @@ function diffChildren( ); if ( - parentNode.nodeName === '#document' && + oldTree.nodeName === '#document' && 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. @@ -453,14 +436,13 @@ function diffChildren( (newNode.nodeType === newNode.ELEMENT_NODE && oldStartNode.nodeType === oldStartNode.ELEMENT_NODE)) ) { - parentNode.removeChild(oldStartNode); + oldTree.removeChild(oldStartNode); replayer.mirror.removeNodeFromMap(oldStartNode); oldStartNode = oldChildren[++oldStartIndex]; } try { - parentNode.insertBefore(newNode, oldStartNode || null); - diff(newNode, newStartNode, replayer, rrnodeMirror); + oldTree.insertBefore(newNode, oldStartNode || null); } catch (e) { console.warn(e); } @@ -482,8 +464,7 @@ function diffChildren( rrnodeMirror, ); try { - parentNode.insertBefore(newNode, referenceNode); - diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror); + oldTree.insertBefore(newNode, referenceNode); } catch (e) { console.warn(e); } @@ -491,15 +472,24 @@ function diffChildren( } else if (newStartIndex > newEndIndex) { for (; oldStartIndex <= oldEndIndex; oldStartIndex++) { const node = oldChildren[oldStartIndex]; - if (!node || node.parentNode !== parentNode) continue; + if (!node || node.parentNode !== oldTree) continue; try { - parentNode.removeChild(node); + oldTree.removeChild(node); replayer.mirror.removeNodeFromMap(node); } catch (e) { console.warn(e); } } } + + // Recursively diff the children of the old tree and the new tree with their props and deeper structures. + let oldChild = oldTree.firstChild; + let newChild = newTree.firstChild; + while (oldChild !== null && newChild !== null) { + diff(oldChild, newChild, replayer, rrnodeMirror); + oldChild = oldChild.nextSibling; + newChild = newChild.nextSibling; + } } export function createOrGetNode( diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 04d638c314..3f18a6ee7e 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -15,6 +15,7 @@ import { Mirror as RRNodeMirror, RRDocument, RRMediaElement, + printRRDom, } from '../src'; import { createOrGetNode, @@ -106,6 +107,7 @@ function shuffle(list: number[]) { describe('diff algorithm for rrdom', () => { let mirror: NodeMirror; let replayer: ReplayerHandler; + let warn: jest.SpyInstance; beforeEach(() => { mirror = createMirror(); @@ -118,6 +120,14 @@ describe('diff algorithm for rrdom', () => { afterAppend: () => {}, }; document.write(''); + // Mock the original console.warn function to make the test fail once console.warn is called. + warn = jest.spyOn(console, 'warn'); + }); + + afterEach(() => { + // Check that warn was not called (fail on warning) + expect(warn).not.toBeCalled(); + warn.mockRestore(); }); describe('diff single node', () => { @@ -437,6 +447,19 @@ describe('diff algorithm for rrdom', () => { expect(document.createElement).toHaveBeenCalledWith('img'); jest.restoreAllMocks(); }); + + it('can omit srcdoc attribute of iframe element', () => { + // If srcdoc attribute is set, the content of iframe recorded by rrweb will be override. + const element = document.createElement('iframe'); + const rrDocument = new RRDocument(); + const rrIframe = rrDocument.createElement('iframe'); + const sn = Object.assign({}, elementSn, { tagName: 'iframe' }); + rrDocument.mirror.add(rrIframe, sn); + rrIframe.attributes['srcdoc'] = ''; + + diff(element, rrIframe, replayer); + expect(element.getAttribute('srcdoc')).toBe(null); + }); }); describe('diff children', () => { @@ -1054,6 +1077,57 @@ describe('diff algorithm for rrdom', () => { const liChild = spanChild.childNodes[0] as HTMLElement; expect(liChild.tagName).toEqual('LI'); }); + + it('should handle corner case with children removed during diff process', () => { + /** + * This test case is to simulate the following scenario: + * The old tree structure: + * 0 P + * 1 SPAN + * 2 SPAN + * The new tree structure: + * 0 P + * 1 SPAN + * 2 SPAN + * 3 SPAN + */ + const node = createTree( + { + tagName: 'p', + id: 0, + children: [1, 2].map((c) => ({ tagName: 'span', id: c })), + }, + undefined, + mirror, + ) as Node; + expect(node.childNodes.length).toEqual(2); + const rrdom = new RRDocument(); + const rrNode = createTree( + { + tagName: 'p', + id: 0, + children: [ + { tagName: 'span', id: 1, children: [{ tagName: 'span', id: 2 }] }, + { tagName: 'span', id: 3 }, + ], + }, + rrdom, + ) as RRNode; + expect(printRRDom(rrNode, rrdom.mirror)).toMatchInlineSnapshot(` + "0 P + 1 SPAN + 2 SPAN + 3 SPAN + " + `); + diff(node, rrNode, replayer); + + expect(node.childNodes.length).toEqual(2); + expect(node.childNodes[0].childNodes.length).toEqual(1); + expect(mirror.getId(node.childNodes[1])).toEqual(3); + expect(node.childNodes[0].childNodes.length).toEqual(1); + expect(mirror.getId(node.childNodes[0].childNodes[0])).toEqual(2); + }); }); describe('diff shadow dom', () => {