Fix rrdom bugs

.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.

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,38 +359,34 @@ 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 (
       // is the first old node the same as the last new node?
       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 (
       // is the last old node the same as the first new node?
       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,24 +464,32 @@ function diffChildren(
         rrnodeMirror,
       );
       try {
-        parentNode.insertBefore(newNode, referenceNode);
-        diff(newNode, newChildren[newStartIndex], replayer, rrnodeMirror);
+        oldTree.insertBefore(newNode, referenceNode);
       } catch (e) {
         console.warn(e);
       }
     }
   } 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(

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('<!DOCTYPE html><html><head></head><body></body></html>');
+    // 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'] = '<html></html>';
+
+      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', () => {