Skip to content

Commit bc4ab9e

Browse files
juliechengjuliecheng
committed
fix: skip calling setAttribute on unchanged attr in diffProps
1 parent fd9d274 commit bc4ab9e

File tree

5 files changed

+208
-1
lines changed

5 files changed

+208
-1
lines changed

‎.changeset/new-turkeys-compare.md

+5
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

+3-1
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

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

493519
describe('diff children', () => {

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

+153
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

+21
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 orderingEvents from './events/ordering';
1617
import scrollEvents from './events/scroll';
1718
import scrollWithParentStylesEvents from './events/scroll-with-parent-styles';
@@ -299,6 +300,26 @@ describe('replayer', function () {
299300
expect(result).toEqual(false);
300301
});
301302

303+
it('should persist inserted stylesheet rules on fast forward', async () => {
304+
await page.evaluate(`events = ${JSON.stringify(insertedStyleSheetRuleEvents)}`);
305+
const result = await page.evaluate(`
306+
const { Replayer } = rrweb;
307+
const replayer = new Replayer(events);
308+
replayer.pause(0);
309+
310+
// fast forward past the insertedRules at 500 and other mutations at 900
311+
replayer.pause(905);
312+
313+
const allStylesheets = Array.from(replayer.iframe.contentDocument.styleSheets);
314+
// find the stylesheet that has the cssRule with the selectorText '.original-style-rule'
315+
const stylesheetWithInsertedRule = allStylesheets.find(sheet => Array.from(sheet.cssRules).find(rule => rule.selectorText === '.original-style-rule'));
316+
stylesheetWithInsertedRule.cssRules.length;
317+
`);
318+
319+
// verify the inserted cssRule wasn't dropped
320+
expect(result).toEqual(2);
321+
});
322+
302323
it('should apply fast-forwarded StyleSheetRules that came after stylesheet textContent overwrite', async () => {
303324
await page.evaluate(`events = ${JSON.stringify(styleSheetRuleEvents)}`);
304325
const result = await page.evaluate(`

0 commit comments

Comments
 (0)