Skip to content

Commit b43dc59

Browse files
committed
fix: skip calling setAttribute on unchanged attr in diffProps
1 parent 53e15c4 commit b43dc59

File tree

5 files changed

+208
-1
lines changed

5 files changed

+208
-1
lines changed

.changeset/new-turkeys-compare.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"rrdom": patch
3+
---
4+
5+
In Chrome, calling setAttribute('type', 'text/css') on style elements that already have it causes Chrome to drop CSS rules that were previously added to the stylesheet via insertRule. This fix prevents setAttribute from being called if the attribute already exists.

packages/rrdom/src/diff.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,9 @@ function diffProps(
354354
} else if (newTree.tagName === 'IFRAME' && name === 'srcdoc') continue;
355355
else {
356356
try {
357-
oldTree.setAttribute(name, newValue);
357+
if (oldTree.getAttribute(name) !== newValue) {
358+
oldTree.setAttribute(name, newValue);
359+
}
358360
} catch (err) {
359361
// We want to continue diffing so we quietly catch
360362
// this exception. Otherwise, this can throw and bubble up to

packages/rrdom/test/diff.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,32 @@ describe('diff algorithm for rrdom', () => {
489489
diff(element, rrIframe, replayer);
490490
expect(element.getAttribute('srcdoc')).toBe(null);
491491
});
492+
493+
it('can skip setAttribute when value is unchanged', () => {
494+
const tagName = 'STYLE';
495+
const element = document.createElement(tagName);
496+
const sn = Object.assign({}, elementSn, { tagName });
497+
mirror.add(element, sn);
498+
499+
element.setAttribute('type', 'text/css');
500+
expect(element.getAttribute('type')).toEqual('text/css');
501+
502+
const rrDocument = new RRDocument();
503+
const rrNode = rrDocument.createElement(tagName);
504+
const sn2 = Object.assign({}, elementSn, { tagName });
505+
rrDocument.mirror.add(rrNode, sn2);
506+
rrNode.attributes = { type: 'text/css' };
507+
508+
const setAttributeSpy = vi.spyOn(element, 'setAttribute');
509+
diff(element, rrNode, replayer);
510+
expect(setAttributeSpy).not.toHaveBeenCalled();
511+
512+
rrNode.attributes = { type: 'application/css' };
513+
diff(element, rrNode, replayer);
514+
expect(setAttributeSpy).toHaveBeenCalledWith('type', 'application/css');
515+
516+
setAttributeSpy.mockRestore();
517+
});
492518
});
493519

494520
describe('diff children', () => {
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
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 + 100,
15+
},
16+
{
17+
type: EventType.Meta,
18+
data: {
19+
href: 'http://localhost',
20+
width: 1000,
21+
height: 800,
22+
},
23+
timestamp: now + 100,
24+
},
25+
// full snapshot:
26+
{
27+
data: {
28+
node: {
29+
id: 1,
30+
type: 0,
31+
childNodes: [
32+
{ id: 2, name: 'html', type: 1, publicId: '', systemId: '' },
33+
{
34+
id: 3,
35+
type: 2,
36+
tagName: 'html',
37+
attributes: { lang: 'en' },
38+
childNodes: [
39+
{
40+
id: 4,
41+
type: 2,
42+
tagName: 'head',
43+
attributes: {},
44+
childNodes: [
45+
{
46+
id: 101,
47+
type: 2,
48+
tagName: 'style',
49+
attributes: {
50+
'data-meta': 'from full-snapshot, gets rule added at 500',
51+
},
52+
childNodes: [
53+
{
54+
id: 102,
55+
type: 3,
56+
isStyle: true,
57+
textContent:
58+
'\n.css-added-at-200-overwritten-at-3000 {\n opacity: 1;\n transform: translateX(0);\n}\n',
59+
},
60+
],
61+
},
62+
{
63+
id: 105,
64+
type: 2,
65+
tagName: 'style',
66+
attributes: {
67+
_cssText:
68+
'.css-added-at-200 { position: fixed; top: 0px; right: 0px; left: 4rem; z-index: 15; flex-shrink: 0; height: 0.25rem; overflow: hidden; background-color: rgb(17, 171, 209); }.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease-in 0.1s; }.css-added-at-200.alt2 { padding-left: 4rem; }',
69+
'data-emotion': 'css',
70+
},
71+
childNodes: [
72+
{ id: 106, type: 3, isStyle: true, textContent: '' },
73+
],
74+
},
75+
],
76+
},
77+
{
78+
id: 107,
79+
type: 2,
80+
tagName: 'body',
81+
attributes: {},
82+
childNodes: [
83+
{
84+
id: 108,
85+
type: 2,
86+
tagName: 'style',
87+
attributes: {
88+
'type': 'text/css',
89+
'gs-style-id': 'gs-id-0',
90+
'_cssText': '.original-style-rule { color: red; }'
91+
},
92+
childNodes: [
93+
{
94+
id: 109,
95+
type: 3,
96+
textContent: '',
97+
isStyle: true
98+
},
99+
],
100+
},
101+
],
102+
},
103+
],
104+
},
105+
],
106+
},
107+
initialOffset: { top: 0, left: 0 },
108+
},
109+
type: EventType.FullSnapshot,
110+
timestamp: now + 100,
111+
},
112+
// mutation that uses insertRule to add a new style rule to the existing body stylesheet
113+
{
114+
data: {
115+
id: 108,
116+
adds: [
117+
{
118+
rule: '.css-inserted-at-500 {border: 1px solid blue;}',
119+
index: 1,
120+
},
121+
],
122+
source: IncrementalSource.StyleSheetRule,
123+
},
124+
type: EventType.IncrementalSnapshot,
125+
timestamp: now + 500,
126+
},
127+
// mutation that adds a child to the body
128+
{
129+
type: EventType.IncrementalSnapshot,
130+
data: {
131+
source: IncrementalSource.Mutation,
132+
texts: [],
133+
attributes: [],
134+
removes: [],
135+
adds: [
136+
{
137+
parentId: 107,
138+
nextId: null,
139+
node: {
140+
type: 2,
141+
tagName: 'div',
142+
attributes: {},
143+
childNodes: [],
144+
id: 110,
145+
},
146+
}
147+
],
148+
},
149+
timestamp: now + 900,
150+
}
151+
];
152+
153+
export default events;

packages/rrweb/test/replayer.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
waitForRAF,
1313
} from './utils';
1414
import styleSheetRuleEvents from './events/style-sheet-rule-events';
15+
import inlineStyleSheetRuleEvents from './events/inline-style-sheet-rule-events';
1516
import movingStyleSheetOnDiff from './events/moving-style-sheet-on-diff';
1617
import orderingEvents from './events/ordering';
1718
import scrollEvents from './events/scroll';
@@ -179,6 +180,26 @@ describe('replayer', function () {
179180
await assertDomSnapshot(page);
180181
});
181182

183+
it('should persist inserted StyleSheetRules on fast forward', async () => {
184+
await page.evaluate(`events = ${JSON.stringify(inlineStyleSheetRuleEvents)}`);
185+
const result = await page.evaluate(`
186+
const { Replayer } = rrweb;
187+
const replayer = new Replayer(events);
188+
replayer.pause(0);
189+
190+
// fast forward past the insertedRules at 500 and mutations at 900
191+
replayer.pause(905);
192+
const styleSheets = Array.from(replayer.iframe.contentDocument.styleSheets);
193+
194+
// find the inlineStyleSheet that has the cssRule with the selectorText '.original-style-rule'
195+
const inlineStyleSheet = styleSheets.find(sheet => Array.from(sheet.cssRules).find(rule => rule.selectorText === '.original-style-rule'));
196+
inlineStyleSheet.cssRules.length;
197+
`);
198+
199+
// verify the inserted cssRule didn't get dropped
200+
expect(result).toEqual(2);
201+
});
202+
182203
it('should persist StyleSheetRule changes when skipping triggers parent style element to move in diff', async () => {
183204
await page.evaluate(`events = ${JSON.stringify(movingStyleSheetOnDiff)}`);
184205

0 commit comments

Comments
 (0)