Skip to content

fix: inserted styles lost when moving elements #1357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 66 additions & 5 deletions packages/rrdom/src/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ function diffChildren(
nodeMatching(oldStartNode, newEndNode, replayer.mirror, rrnodeMirror)
) {
try {
oldTree.insertBefore(oldStartNode, oldEndNode.nextSibling);
handleInsertBefore(oldTree, oldStartNode, oldEndNode.nextSibling);
} catch (e) {
console.warn(e);
}
Expand All @@ -424,7 +424,7 @@ function diffChildren(
nodeMatching(oldEndNode, newStartNode, replayer.mirror, rrnodeMirror)
) {
try {
oldTree.insertBefore(oldEndNode, oldStartNode);
handleInsertBefore(oldTree, oldEndNode, oldStartNode);
} catch (e) {
console.warn(e);
}
Expand All @@ -449,7 +449,7 @@ function diffChildren(
nodeMatching(nodeToMove, newStartNode, replayer.mirror, rrnodeMirror)
) {
try {
oldTree.insertBefore(nodeToMove, oldStartNode);
handleInsertBefore(oldTree, nodeToMove, oldStartNode);
} catch (e) {
console.warn(e);
}
Expand Down Expand Up @@ -483,7 +483,7 @@ function diffChildren(
}

try {
oldTree.insertBefore(newNode, oldStartNode || null);
handleInsertBefore(oldTree, newNode, oldStartNode || null);
} catch (e) {
console.warn(e);
}
Expand All @@ -505,7 +505,7 @@ function diffChildren(
rrnodeMirror,
);
try {
oldTree.insertBefore(newNode, referenceNode);
handleInsertBefore(oldTree, newNode, referenceNode);
} catch (e) {
console.warn(e);
}
Expand Down Expand Up @@ -613,3 +613,64 @@ export function nodeMatching(
if (node1Id === -1 || node1Id !== node2Id) return false;
return sameNodeType(node1, node2);
}

/**
* Copies CSSRules and their position from HTML style element which don't exist in it's innerText
*/
function getInsertedStylesFromElement(
styleElement: HTMLStyleElement,
): Array<{ index: number; cssRuleText: string }> | undefined {
const elementCssRules = styleElement.sheet?.cssRules;
if (!elementCssRules || !elementCssRules.length) return;
// style sheet w/ innerText styles to diff with actual and get only inserted styles
const tempStyleSheet = new CSSStyleSheet();
tempStyleSheet.replaceSync(styleElement.innerText);

const innerTextStylesMap: { [key: string]: CSSRule } = {};

for (let i = 0; i < tempStyleSheet.cssRules.length; i++) {
innerTextStylesMap[tempStyleSheet.cssRules[i].cssText] =
tempStyleSheet.cssRules[i];
}

const insertedStylesStyleSheet = [];

for (let i = 0; i < elementCssRules?.length; i++) {
const cssRuleText = elementCssRules[i].cssText;

if (!innerTextStylesMap[cssRuleText]) {
insertedStylesStyleSheet.push({
index: i,
cssRuleText,
});
}
}

return insertedStylesStyleSheet;
}

/**
* Conditionally copy insertedStyles for STYLE nodes and apply after calling insertBefore'
* For non-STYLE nodes, just insertBefore
*/
export function handleInsertBefore(
oldTree: Node,
nodeToMove: Node,
insertBeforeNode: Node | null,
): void {
let insertedStyles;

if (nodeToMove.nodeName === 'STYLE') {
insertedStyles = getInsertedStylesFromElement(
nodeToMove as HTMLStyleElement,
);
}

oldTree.insertBefore(nodeToMove, insertBeforeNode);

if (insertedStyles && insertedStyles.length) {
insertedStyles.forEach(({ cssRuleText, index }) => {
(nodeToMove as HTMLStyleElement).sheet?.insertRule(cssRuleText, index);
});
}
}
70 changes: 68 additions & 2 deletions packages/rrdom/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ReplayerHandler,
nodeMatching,
sameNodeType,
handleInsertBefore,
} from '../src/diff';
import type { IRRElement, IRRNode } from '../src/document';
import type { canvasMutationData, styleSheetRuleData } from '@rrweb/types';
Expand Down Expand Up @@ -1467,7 +1468,7 @@ describe('diff algorithm for rrdom', () => {
const rrHtmlEl = rrDocument.createElement('html');
rrDocument.mirror.add(rrHtmlEl, rrdom.getDefaultSN(rrHtmlEl, ${htmlElId}));
rrIframeEl.contentDocument.appendChild(rrHtmlEl);

const replayer = {
mirror: rrdom.createMirror(),
applyCanvas: () => {},
Expand All @@ -1476,7 +1477,7 @@ describe('diff algorithm for rrdom', () => {
applyStyleSheetMutation: () => {},
};
rrdom.diff(iframeEl, rrIframeEl, replayer);

iframeEl.contentDocument.documentElement.className =
'${className.toLowerCase()}';
iframeEl.contentDocument.childNodes.length === 2 &&
Expand Down Expand Up @@ -1838,4 +1839,69 @@ describe('diff algorithm for rrdom', () => {
expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy();
});
});

describe('test handleInsertBefore function', () => {
it('should insert nodeToMove before insertBeforeNode in oldTree for non-style elements', () => {
const oldTree = document.createElement('div');
const nodeToMove = document.createElement('div');
const insertBeforeNode = document.createElement('div');
oldTree.appendChild(insertBeforeNode);

expect(oldTree.children.length).toEqual(1);

handleInsertBefore(oldTree, nodeToMove, insertBeforeNode);

expect(oldTree.children.length).toEqual(2);
expect(oldTree.children[0]).toEqual(nodeToMove);
});

it('should not drop inserted styles when moving a style element with inserted styles', async () => {
function MockCSSStyleSheet() {
this.replaceSync = vi.fn();
this.cssRules = [{ cssText: baseStyle }];
}

vi.spyOn(window, 'CSSStyleSheet').mockImplementationOnce(
MockCSSStyleSheet as any,
);

const baseStyle = 'body {margin: 0;}';
const insertedStyle = 'div {display: flex;}';

document.write('<html></html>');

const insertBeforeNode = document.createElement('style');
document.documentElement.appendChild(insertBeforeNode);

const nodeToMove = document.createElement('style');
nodeToMove.appendChild(document.createTextNode(baseStyle));
document.documentElement.appendChild(nodeToMove);
nodeToMove.sheet?.insertRule(insertedStyle);

// validate dom prior to moving element
expect(document.documentElement.children.length).toEqual(4);
expect(document.documentElement.children[2]).toEqual(insertBeforeNode);
expect(document.documentElement.children[3]).toEqual(nodeToMove);
expect(nodeToMove.sheet?.cssRules.length).toEqual(2);
expect(nodeToMove.sheet?.cssRules[0].cssText).toEqual(insertedStyle);
expect(nodeToMove.sheet?.cssRules[1].cssText).toEqual(baseStyle);

// move the node
handleInsertBefore(
document.documentElement,
nodeToMove,
insertBeforeNode,
);

// nodeToMove was inserted before
expect(document.documentElement.children.length).toEqual(4);
expect(document.documentElement.children[2]).toEqual(nodeToMove);
expect(document.documentElement.children[3]).toEqual(insertBeforeNode);
// styles persisted on the moved element
// w/ document.documentElement.insertBefore(nodeToMove, insertBeforeNode) insertedStyle wouldn't be copied
expect(nodeToMove.sheet?.cssRules.length).toEqual(2);
expect(nodeToMove.sheet?.cssRules[0].cssText).toEqual(insertedStyle);
expect(nodeToMove.sheet?.cssRules[1].cssText).toEqual(baseStyle);
});
});
});
Loading