Skip to content

Commit

Permalink
Fix rrdom bugs (#1222)
Browse files Browse the repository at this point in the history 
* fix: rrdom bug

1. fix one bug of the diff algorithm
2. omit srcdoc attribute of iframe

* use linked list to iterate children

* fix the bug that the children of shadowRoot don't get diffed

* add test cases

* add change log
  • Loading branch information
@YunFeng0817
YunFeng0817 authored Jun 2, 2023
1 parent a606109 commit b798f2d
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 43 deletions.
8 changes: 8 additions & 0 deletions .changeset/cold-eyes-hunt.md
View file
Original file line number Diff line number Diff line change
@@ -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.
76 changes: 33 additions & 43 deletions packages/rrdom/src/diff.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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))
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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);
}
Expand All @@ -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(
Expand Down
74 changes: 74 additions & 0 deletions packages/rrdom/test/diff.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Mirror as RRNodeMirror,
RRDocument,
RRMediaElement,
printRRDom,
} from '../src';
import {
createOrGetNode,
Expand Down Expand Up @@ -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();
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit b798f2d

Please sign in to comment.