Skip to content

Commit 9c4d539

Browse files
author
jaj1014
committed
fix: inserted styles lost when moving elements
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues
1 parent 8aea5b0 commit 9c4d539

File tree

3 files changed

+291
-9
lines changed

3 files changed

+291
-9
lines changed

‎packages/rrdom/src/diff.ts

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ function diffChildren(
372372
nodeMatching(oldStartNode, newEndNode, replayer.mirror, rrnodeMirror)
373373
) {
374374
try {
375-
oldTree.insertBefore(oldStartNode, oldEndNode.nextSibling);
375+
handleInsertBefore(oldTree, oldStartNode, oldEndNode.nextSibling);
376376
} catch (e) {
377377
console.warn(e);
378378
}
@@ -383,7 +383,7 @@ function diffChildren(
383383
nodeMatching(oldEndNode, newStartNode, replayer.mirror, rrnodeMirror)
384384
) {
385385
try {
386-
oldTree.insertBefore(oldEndNode, oldStartNode);
386+
handleInsertBefore(oldTree, oldEndNode, oldStartNode);
387387
} catch (e) {
388388
console.warn(e);
389389
}
@@ -408,7 +408,7 @@ function diffChildren(
408408
nodeMatching(nodeToMove, newStartNode, replayer.mirror, rrnodeMirror)
409409
) {
410410
try {
411-
oldTree.insertBefore(nodeToMove, oldStartNode);
411+
handleInsertBefore(oldTree, nodeToMove, oldStartNode);
412412
} catch (e) {
413413
console.warn(e);
414414
}
@@ -442,7 +442,7 @@ function diffChildren(
442442
}
443443

444444
try {
445-
oldTree.insertBefore(newNode, oldStartNode || null);
445+
handleInsertBefore(oldTree, newNode, oldStartNode || null);
446446
} catch (e) {
447447
console.warn(e);
448448
}
@@ -464,7 +464,8 @@ function diffChildren(
464464
rrnodeMirror,
465465
);
466466
try {
467-
oldTree.insertBefore(newNode, referenceNode);
467+
// oldTree.insertBefore(newNode, referenceNode);
468+
handleInsertBefore(oldTree, newNode, referenceNode);
468469
} catch (e) {
469470
console.warn(e);
470471
}
@@ -572,3 +573,64 @@ export function nodeMatching(
572573
if (node1Id === -1 || node1Id !== node2Id) return false;
573574
return sameNodeType(node1, node2);
574575
}
576+
577+
/**
578+
* Copies CSSRules and their position from HTML style element which don't exist in it's innerText
579+
*/
580+
export function getInsertedStylesFromElement(
581+
styleElement: HTMLStyleElement,
582+
): Array<{ index: number; cssRuleText: string }> | undefined {
583+
const elementCssRules = styleElement.sheet?.cssRules;
584+
if (!elementCssRules || !elementCssRules.length) return;
585+
// style sheet w/ innerText styles to diff with actual and get only inserted styles
586+
const tempStyleSheet = new CSSStyleSheet();
587+
tempStyleSheet.replaceSync(styleElement.innerText);
588+
589+
const innerTextStylesMap: { [key: string]: CSSRule } = {};
590+
591+
for (let i = 0; i < tempStyleSheet.cssRules.length; i++) {
592+
innerTextStylesMap[tempStyleSheet.cssRules[i].cssText] =
593+
tempStyleSheet.cssRules[i];
594+
}
595+
596+
const insertedStylesStyleSheet = [];
597+
598+
for (let i = 0; i < elementCssRules?.length; i++) {
599+
const cssRuleText = elementCssRules[i].cssText;
600+
601+
if (!innerTextStylesMap[cssRuleText]) {
602+
insertedStylesStyleSheet.push({
603+
index: i,
604+
cssRuleText,
605+
});
606+
}
607+
}
608+
609+
return insertedStylesStyleSheet;
610+
}
611+
612+
/**
613+
* Conditionally copy insertedStyles for STYLE nodes and apply after calling insertBefore'
614+
* For non-STYLE nodes, just insertBefore
615+
*/
616+
function handleInsertBefore(
617+
oldTree: Node,
618+
nodeToMove: Node,
619+
insertBeforeNode: Node | null,
620+
): void {
621+
let insertedStyles;
622+
623+
if (nodeToMove.nodeName === 'STYLE') {
624+
insertedStyles = getInsertedStylesFromElement(
625+
nodeToMove as HTMLStyleElement,
626+
);
627+
}
628+
629+
oldTree.insertBefore(nodeToMove, insertBeforeNode);
630+
631+
if (insertedStyles && insertedStyles.length) {
632+
insertedStyles.forEach(({ cssRuleText, index }) => {
633+
(nodeToMove as HTMLStyleElement).sheet?.insertRule(cssRuleText, index);
634+
});
635+
}
636+
}

‎packages/rrweb/test/events/moving-style-sheet-on-diff.ts

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import { EventType, IncrementalSource } from '@rrweb/types';
2+
import type { eventWithTime } from '@rrweb/types';
3+
4+
const now = Date.now();
5+
const events: eventWithTime[] = [
6+
{
7+
type: EventType.DomContentLoaded,
8+
data: {},
9+
timestamp: now,
10+
},
11+
{
12+
type: EventType.Load,
13+
data: {},
14+
timestamp: now + 10,
15+
},
16+
{
17+
type: EventType.Meta,
18+
data: {
19+
href: 'http://localhost',
20+
width: 1000,
21+
height: 800,
22+
},
23+
timestamp: now + 10,
24+
},
25+
// full snapshot:
26+
{
27+
data: {
28+
node: {
29+
type: 0,
30+
childNodes: [
31+
{
32+
type: 1,
33+
name: 'html',
34+
publicId: '',
35+
systemId: '',
36+
id: 2,
37+
},
38+
{
39+
type: 2,
40+
tagName: 'html',
41+
attributes: {
42+
lang: 'en',
43+
},
44+
childNodes: [
45+
{
46+
type: 2,
47+
tagName: 'head',
48+
attributes: {},
49+
childNodes: [
50+
{
51+
type: 2,
52+
tagName: 'style',
53+
attributes: {},
54+
childNodes: [
55+
{
56+
type: 3,
57+
textContent:
58+
'#wrapper { width: 200px; margin: 50px auto; background-color: gainsboro; padding: 20px; }.target-element { padding: 12px; margin-top: 12px; }',
59+
isStyle: true,
60+
id: 6,
61+
},
62+
],
63+
id: 5,
64+
},
65+
{
66+
type: 2,
67+
tagName: 'style',
68+
attributes: {},
69+
childNodes: [
70+
{
71+
type: 3,
72+
textContent:
73+
'.new-element-class { font-size: 32px; color: tomato; }',
74+
isStyle: true,
75+
id: 8,
76+
},
77+
],
78+
id: 7,
79+
},
80+
],
81+
id: 4,
82+
},
83+
{
84+
type: 2,
85+
tagName: 'body',
86+
attributes: {},
87+
childNodes: [
88+
{
89+
type: 2,
90+
tagName: 'div',
91+
attributes: {
92+
id: 'wrapper',
93+
},
94+
childNodes: [
95+
{
96+
type: 2,
97+
tagName: 'div',
98+
attributes: {
99+
class: 'target-element',
100+
},
101+
childNodes: [
102+
{
103+
type: 2,
104+
tagName: 'p',
105+
attributes: {
106+
class: 'target-element-child',
107+
},
108+
childNodes: [
109+
{
110+
type: 3,
111+
textContent: 'Element to style',
112+
id: 113,
113+
},
114+
],
115+
id: 12,
116+
},
117+
],
118+
id: 11,
119+
},
120+
],
121+
id: 10,
122+
},
123+
],
124+
id: 9,
125+
},
126+
],
127+
id: 3,
128+
},
129+
],
130+
id: 1,
131+
},
132+
initialOffset: {
133+
left: 0,
134+
top: 0,
135+
},
136+
},
137+
type: EventType.FullSnapshot,
138+
timestamp: now + 20,
139+
},
140+
// 1st mutation that applies StyleSheetRule
141+
{
142+
type: EventType.IncrementalSnapshot,
143+
data: {
144+
source: IncrementalSource.StyleSheetRule,
145+
id: 5,
146+
adds: [
147+
{
148+
rule: '.target-element{background-color:teal;}',
149+
},
150+
],
151+
},
152+
timestamp: now + 30,
153+
},
154+
// 2nd mutation inserts new style element to trigger other style element to get moved in diff
155+
{
156+
type: EventType.IncrementalSnapshot,
157+
data: {
158+
source: IncrementalSource.Mutation,
159+
texts: [],
160+
attributes: [],
161+
removes: [{ parentId: 4, id: 7 }],
162+
adds: [
163+
{
164+
parentId: 4,
165+
nextId: 5,
166+
node: {
167+
type: 2,
168+
tagName: 'style',
169+
attributes: {},
170+
childNodes: [],
171+
id: 98,
172+
},
173+
},
174+
{
175+
parentId: 98,
176+
nextId: null,
177+
node: {
178+
type: 3,
179+
textContent:
180+
'.new-element-class { font-size: 32px; color: tomato; }',
181+
isStyle: true,
182+
id: 99,
183+
},
184+
},
185+
],
186+
},
187+
timestamp: now + 2000,
188+
},
189+
// dummy event to have somewhere to skip
190+
{
191+
data: {
192+
adds: [],
193+
texts: [],
194+
source: IncrementalSource.Mutation,
195+
removes: [],
196+
attributes: [],
197+
},
198+
type: EventType.IncrementalSnapshot,
199+
timestamp: now + 3000,
200+
},
201+
];
202+
203+
export default events;

‎packages/rrweb/test/replayer.test.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
waitForRAF,
1111
} from './utils';
1212
import styleSheetRuleEvents from './events/style-sheet-rule-events';
13+
import movingStyleSheetOnDiff from './events/moving-style-sheet-on-diff';
1314
import orderingEvents from './events/ordering';
1415
import scrollEvents from './events/scroll';
1516
import inputEvents from './events/input';
@@ -173,6 +174,22 @@ describe('replayer', function () {
173174
await assertDomSnapshot(page);
174175
});
175176

177+
it('should persist StyleSheetRule changes when skipping triggers parent style element to move in diff', async () => {
178+
await page.evaluate(`events = ${JSON.stringify(movingStyleSheetOnDiff)}`);
179+
180+
const result = await page.evaluate(`
181+
const { Replayer } = rrweb;
182+
const replayer = new Replayer(events);
183+
replayer.pause(3000);
184+
const rules = [...replayer.iframe.contentDocument.styleSheets].map(
185+
(sheet) => [...sheet.rules],
186+
).flat();
187+
rules.some((x) => x.cssText === '.target-element { background-color: teal; }');
188+
`);
189+
190+
expect(result).toEqual(true);
191+
});
192+
176193
it('should apply fast forwarded StyleSheetRules that where added', async () => {
177194
await page.evaluate(`events = ${JSON.stringify(styleSheetRuleEvents)}`);
178195
const result = await page.evaluate(`
@@ -224,7 +241,7 @@ describe('replayer', function () {
224241
await waitForRAF(page);
225242

226243
/** check the second selection event */
227-
[startOffset, endOffset] = (await page.evaluate(`
244+
[startOffset, endOffset] = (await page.evaluate(`
228245
replayer.pause(410);
229246
var range = replayer.iframe.contentDocument.getSelection().getRangeAt(0);
230247
[range.startOffset, range.endOffset];
@@ -656,7 +673,7 @@ describe('replayer', function () {
656673
events = ${JSON.stringify(canvasInIframe)};
657674
const { Replayer } = rrweb;
658675
var replayer = new Replayer(events,{showDebug:true});
659-
replayer.pause(550);
676+
replayer.pause(550);
660677
`);
661678
const replayerIframe = await page.$('iframe');
662679
const contentDocument = await replayerIframe!.contentFrame()!;
@@ -718,7 +735,7 @@ describe('replayer', function () {
718735
const replayer = new Replayer(events);
719736
replayer.play();
720737
`);
721-
await page.waitForTimeout(50);
738+
await page.waitForTimeout(150);
722739

723740
await assertDomSnapshot(page);
724741
});
@@ -742,7 +759,7 @@ describe('replayer', function () {
742759
await page.evaluate(`
743760
const { Replayer } = rrweb;
744761
let replayer = new Replayer(events);
745-
replayer.play();
762+
replayer.play();
746763
`);
747764

748765
const replayerWrapperClassName = 'replayer-wrapper';

0 commit comments

Comments
 (0)