From bc4ab9ebd2b974055c3b93461a54a6d599d4c6b6 Mon Sep 17 00:00:00 2001 From: juliecheng Date: Tue, 13 May 2025 17:10:39 -0400 Subject: [PATCH] fix: skip calling setAttribute on unchanged attr in diffProps --- .changeset/new-turkeys-compare.md | 5 + packages/rrdom/src/diff.ts | 4 +- packages/rrdom/test/diff.test.ts | 26 +++ .../events/inserted-stylesheet-rule-events.ts | 153 ++++++++++++++++++ packages/rrweb/test/replayer.test.ts | 21 +++ 5 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 .changeset/new-turkeys-compare.md create mode 100644 packages/rrweb/test/events/inserted-stylesheet-rule-events.ts diff --git a/.changeset/new-turkeys-compare.md b/.changeset/new-turkeys-compare.md new file mode 100644 index 0000000000..cc89c99ac8 --- /dev/null +++ b/.changeset/new-turkeys-compare.md @@ -0,0 +1,5 @@ +--- +"rrdom": patch +--- + +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. diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 5d9ba7cea5..5c7c437226 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -354,7 +354,9 @@ function diffProps( } else if (newTree.tagName === 'IFRAME' && name === 'srcdoc') continue; else { try { - oldTree.setAttribute(name, newValue); + if (oldTree.getAttribute(name) !== newValue) { + oldTree.setAttribute(name, newValue); + } } catch (err) { // We want to continue diffing so we quietly catch // this exception. Otherwise, this can throw and bubble up to diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 0b13edb49c..aa137b236c 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -488,6 +488,32 @@ describe('diff algorithm for rrdom', () => { diff(element, rrIframe, replayer); expect(element.getAttribute('srcdoc')).toBe(null); }); + + it('can skip calling setAttribute when attr/value already exists', () => { + const tagName = 'STYLE'; + const element = document.createElement(tagName); + const sn = Object.assign({}, elementSn, { tagName }); + mirror.add(element, sn); + + element.setAttribute('type', 'text/css'); + expect(element.getAttribute('type')).toEqual('text/css'); + + const rrDocument = new RRDocument(); + const rrNode = rrDocument.createElement(tagName); + const sn2 = Object.assign({}, elementSn, { tagName }); + rrDocument.mirror.add(rrNode, sn2); + rrNode.attributes = { type: 'text/css' }; + + const setAttributeSpy = vi.spyOn(element, 'setAttribute'); + diff(element, rrNode, replayer); + expect(setAttributeSpy).not.toHaveBeenCalled(); + + rrNode.attributes = { type: 'application/css' }; + diff(element, rrNode, replayer); + expect(setAttributeSpy).toHaveBeenCalledWith('type', 'application/css'); + + setAttributeSpy.mockRestore(); + }); }); describe('diff children', () => { diff --git a/packages/rrweb/test/events/inserted-stylesheet-rule-events.ts b/packages/rrweb/test/events/inserted-stylesheet-rule-events.ts new file mode 100644 index 0000000000..89cc5a164f --- /dev/null +++ b/packages/rrweb/test/events/inserted-stylesheet-rule-events.ts @@ -0,0 +1,153 @@ +import { EventType, IncrementalSource } from '@rrweb/types'; +import type { eventWithTime } from '@rrweb/types'; + +const now = Date.now(); +const events: eventWithTime[] = [ + { + type: EventType.DomContentLoaded, + data: {}, + timestamp: now, + }, + { + type: EventType.Load, + data: {}, + timestamp: now + 100, + }, + { + type: EventType.Meta, + data: { + href: 'http://localhost', + width: 1000, + height: 800, + }, + timestamp: now + 100, + }, + // full snapshot: + { + data: { + node: { + id: 1, + type: 0, + childNodes: [ + { id: 2, name: 'html', type: 1, publicId: '', systemId: '' }, + { + id: 3, + type: 2, + tagName: 'html', + attributes: { lang: 'en' }, + childNodes: [ + { + id: 4, + type: 2, + tagName: 'head', + attributes: {}, + childNodes: [ + { + id: 101, + type: 2, + tagName: 'style', + attributes: { + 'data-meta': 'from full-snapshot, gets rule added at 500', + }, + childNodes: [ + { + id: 102, + type: 3, + isStyle: true, + textContent: + '\n.css-added-at-200-overwritten-at-3000 {\n opacity: 1;\n transform: translateX(0);\n}\n', + }, + ], + }, + { + id: 105, + type: 2, + tagName: 'style', + attributes: { + _cssText: + '.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; }', + 'data-emotion': 'css', + }, + childNodes: [ + { id: 106, type: 3, isStyle: true, textContent: '' }, + ], + }, + ], + }, + { + id: 107, + type: 2, + tagName: 'body', + attributes: {}, + childNodes: [ + { + id: 108, + type: 2, + tagName: 'style', + attributes: { + 'type': 'text/css', + 'gs-style-id': 'gs-id-0', + '_cssText': '.original-style-rule { color: red; }' + }, + childNodes: [ + { + id: 109, + type: 3, + textContent: '', + isStyle: true + }, + ], + }, + ], + }, + ], + }, + ], + }, + initialOffset: { top: 0, left: 0 }, + }, + type: EventType.FullSnapshot, + timestamp: now + 100, + }, + // mutation that uses insertRule to add a new style rule to the existing body stylesheet + { + data: { + id: 108, + adds: [ + { + rule: '.css-inserted-at-500 {border: 1px solid blue;}', + index: 1, + }, + ], + source: IncrementalSource.StyleSheetRule, + }, + type: EventType.IncrementalSnapshot, + timestamp: now + 500, + }, + // mutation that adds a child to the body + { + type: EventType.IncrementalSnapshot, + data: { + source: IncrementalSource.Mutation, + texts: [], + attributes: [], + removes: [], + adds: [ + { + parentId: 107, + nextId: null, + node: { + type: 2, + tagName: 'div', + attributes: {}, + childNodes: [], + id: 110, + }, + } + ], + }, + timestamp: now + 900, + } +]; + +export default events; diff --git a/packages/rrweb/test/replayer.test.ts b/packages/rrweb/test/replayer.test.ts index c38ec356da..8dab2b86df 100644 --- a/packages/rrweb/test/replayer.test.ts +++ b/packages/rrweb/test/replayer.test.ts @@ -12,6 +12,7 @@ import { waitForRAF, } from './utils'; import styleSheetRuleEvents from './events/style-sheet-rule-events'; +import insertedStyleSheetRuleEvents from './events/inserted-stylesheet-rule-events'; import orderingEvents from './events/ordering'; import scrollEvents from './events/scroll'; import scrollWithParentStylesEvents from './events/scroll-with-parent-styles'; @@ -299,6 +300,26 @@ describe('replayer', function () { expect(result).toEqual(false); }); + it('should persist inserted stylesheet rules on fast forward', async () => { + await page.evaluate(`events = ${JSON.stringify(insertedStyleSheetRuleEvents)}`); + const result = await page.evaluate(` + const { Replayer } = rrweb; + const replayer = new Replayer(events); + replayer.pause(0); + + // fast forward past the insertedRules at 500 and other mutations at 900 + replayer.pause(905); + + const allStylesheets = Array.from(replayer.iframe.contentDocument.styleSheets); + // find the stylesheet that has the cssRule with the selectorText '.original-style-rule' + const stylesheetWithInsertedRule = allStylesheets.find(sheet => Array.from(sheet.cssRules).find(rule => rule.selectorText === '.original-style-rule')); + stylesheetWithInsertedRule.cssRules.length; + `); + + // verify the inserted cssRule wasn't dropped + expect(result).toEqual(2); + }); + it('should apply fast-forwarded StyleSheetRules that came after stylesheet textContent overwrite', async () => { await page.evaluate(`events = ${JSON.stringify(styleSheetRuleEvents)}`); const result = await page.evaluate(`