Skip to content

Commit d3263fc

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

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 calling setAttribute when attr/value already exists', () => {
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', () => {

‎packages/rrweb/test/events/inserted-stylesheet-rule-events.ts

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 insertedStyleSheetRuleEvents from './events/inserted-stylesheet-rule-events';
1516
import movingStyleSheetOnDiff from './events/moving-style-sheet-on-diff';
1617
import orderingEvents from './events/ordering';
1718
import scrollEvents from './events/scroll';
@@ -316,6 +317,26 @@ describe('replayer', function () {
316317
expect(result).toEqual(false);
317318
});
318319

320+
it('should persist inserted stylesheet rules on fast forward', async () => {
321+
await page.evaluate(`events = ${JSON.stringify(insertedStyleSheetRuleEvents)}`);
322+
const result = await page.evaluate(`
323+
const { Replayer } = rrweb;
324+
const replayer = new Replayer(events);
325+
replayer.pause(0);
326+
327+
// fast forward past the insertedRules at 500 and other mutations at 900
328+
replayer.pause(905);
329+
330+
const allStylesheets = Array.from(replayer.iframe.contentDocument.styleSheets);
331+
// find the stylesheet that has the cssRule with the selectorText '.original-style-rule'
332+
const stylesheetWithInsertedRule = allStylesheets.find(sheet => Array.from(sheet.cssRules).find(rule => rule.selectorText === '.original-style-rule'));
333+
stylesheetWithInsertedRule.cssRules.length;
334+
`);
335+
336+
// verify the inserted cssRule wasn't dropped
337+
expect(result).toEqual(2);
338+
});
339+
319340
it('should apply fast-forwarded StyleSheetRules that came after stylesheet textContent overwrite', async () => {
320341
await page.evaluate(`events = ${JSON.stringify(styleSheetRuleEvents)}`);
321342
const result = await page.evaluate(`

0 commit comments

Comments
 (0)