diff --git a/.changeset/clean-plants-play.md b/.changeset/clean-plants-play.md new file mode 100644 index 00000000..809dae8d --- /dev/null +++ b/.changeset/clean-plants-play.md @@ -0,0 +1,9 @@ +--- +'rrweb': patch +'@rrweb/types': patch +--- + +Compact style mutation fixes and improvements + +- fixes when style updates contain a 'var()' on a shorthand property #1246 +- further ensures that style mutations are compact by reverting to string method if it is shorter diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index 39a61076..0eea35c0 100644 --- a/packages/rrweb/src/record/mutation.ts +++ b/packages/rrweb/src/record/mutation.ts @@ -18,7 +18,6 @@ import type { attributeCursor, removedNodeMutation, addedNodeMutation, - styleAttributeValue, Optional, } from '@rrweb/types'; import { @@ -438,10 +437,29 @@ export default class MutationBuffer { // text mutation's id was not in the mirror map means the target node has been removed .filter((text) => this.mirror.has(text.id)), attributes: this.attributes - .map((attribute) => ({ - id: this.mirror.getId(attribute.node), - attributes: attribute.attributes, - })) + .map((attribute) => { + const { attributes } = attribute; + if (typeof attributes.style === 'string') { + const diffAsStr = JSON.stringify(attribute.styleDiff); + const unchangedAsStr = JSON.stringify(attribute._unchangedStyles); + // check if the style diff is actually shorter than the regular string based mutation + // (which was the whole point of #464 'compact style mutation'). + if (diffAsStr.length < attributes.style.length) { + // also: CSSOM fails badly when var() is present on shorthand properties, so only proceed with + // the compact style mutation if these have all been accounted for + if ( + (diffAsStr + unchangedAsStr).split('var(').length === + attributes.style.split('var(').length + ) { + attributes.style = attribute.styleDiff; + } + } + } + return { + id: this.mirror.getId(attribute.node), + attributes: attributes, + }; + }) // attribute mutation's id was not in the mirror map means the target node has been removed .filter((attribute) => this.mirror.has(attribute.id)), removes: this.removes, @@ -548,6 +566,8 @@ export default class MutationBuffer { item = { node: m.target, attributes: {}, + styleDiff: {}, + _unchangedStyles: {}, }; this.attributes.push(item); } @@ -562,39 +582,7 @@ export default class MutationBuffer { target.setAttribute('data-rr-is-password', 'true'); } - if (attributeName === 'style') { - const old = unattachedDoc.createElement('span'); - if (m.oldValue) { - old.setAttribute('style', m.oldValue); - } - if ( - item.attributes.style === undefined || - item.attributes.style === null - ) { - item.attributes.style = {}; - } - const styleObj = item.attributes.style as styleAttributeValue; - for (const pname of Array.from(target.style)) { - const newValue = target.style.getPropertyValue(pname); - const newPriority = target.style.getPropertyPriority(pname); - if ( - newValue !== old.style.getPropertyValue(pname) || - newPriority !== old.style.getPropertyPriority(pname) - ) { - if (newPriority === '') { - styleObj[pname] = newValue; - } else { - styleObj[pname] = [newValue, newPriority]; - } - } - } - for (const pname of Array.from(old.style)) { - if (target.style.getPropertyValue(pname) === '') { - // "if not set, returns the empty string" - styleObj[pname] = false; // delete - } - } - } else if (!ignoreAttribute(target.tagName, attributeName, value)) { + if (!ignoreAttribute(target.tagName, attributeName, value)) { // overwrite attribute if the mutations was triggered in same time item.attributes[attributeName] = transformAttribute( this.doc, @@ -602,6 +590,35 @@ export default class MutationBuffer { toLowerCase(attributeName), value, ); + if (attributeName === 'style') { + const old = unattachedDoc.createElement('span'); + if (m.oldValue) { + old.setAttribute('style', m.oldValue); + } + for (const pname of Array.from(target.style)) { + const newValue = target.style.getPropertyValue(pname); + const newPriority = target.style.getPropertyPriority(pname); + if ( + newValue !== old.style.getPropertyValue(pname) || + newPriority !== old.style.getPropertyPriority(pname) + ) { + if (newPriority === '') { + item.styleDiff[pname] = newValue; + } else { + item.styleDiff[pname] = [newValue, newPriority]; + } + } else { + // for checking + item._unchangedStyles[pname] = [newValue, newPriority]; + } + } + for (const pname of Array.from(old.style)) { + if (target.style.getPropertyValue(pname) === '') { + // "if not set, returns the empty string" + item.styleDiff[pname] = false; // delete + } + } + } } break; } diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index f244948b..773d2351 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -2933,24 +2933,14 @@ exports[`record integration tests can record node mutations 1`] = ` \\"id\\": 36, \\"attributes\\": { \\"id\\": \\"select2-drop\\", - \\"style\\": { - \\"left\\": \\"Npx\\", - \\"width\\": \\"Npx\\", - \\"top\\": \\"Npx\\", - \\"bottom\\": \\"auto\\", - \\"display\\": \\"block\\", - \\"position\\": false, - \\"visibility\\": false - }, + \\"style\\": \\"left: Npx; width: Npx; top: Npx; bottom: auto; display: block;\\", \\"class\\": \\"select2-drop select2-display-none select2-with-searchbox select2-drop-active\\" } }, { \\"id\\": 70, \\"attributes\\": { - \\"style\\": { - \\"display\\": false - } + \\"style\\": \\"\\" } }, { @@ -3391,6 +3381,232 @@ exports[`record integration tests can record node mutations 1`] = ` ]" `; +exports[`record integration tests can record style changes compactly and preserve css var() functions 1`] = ` +"[ + { + \\"type\\": 0, + \\"data\\": {} + }, + { + \\"type\\": 1, + \\"data\\": {} + }, + { + \\"type\\": 4, + \\"data\\": { + \\"href\\": \\"about:blank\\", + \\"width\\": 1920, + \\"height\\": 1080 + } + }, + { + \\"type\\": 2, + \\"data\\": { + \\"node\\": { + \\"type\\": 0, + \\"childNodes\\": [ + { + \\"type\\": 2, + \\"tagName\\": \\"html\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 2, + \\"tagName\\": \\"head\\", + \\"attributes\\": {}, + \\"childNodes\\": [], + \\"id\\": 3 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"body\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 5 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"script\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"SCRIPT_PLACEHOLDER\\", + \\"id\\": 7 + } + ], + \\"id\\": 6 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\\\n \\\\n\\\\n\\", + \\"id\\": 8 + } + ], + \\"id\\": 4 + } + ], + \\"id\\": 2 + } + ], + \\"compatMode\\": \\"BackCompat\\", + \\"id\\": 1 + }, + \\"initialOffset\\": { + \\"left\\": 0, + \\"top\\": 0 + } + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": \\"background: var(--mystery)\\" + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": \\"background: var(--mystery); background-color: black\\" + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": \\"\\" + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": \\"display:block\\" + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": { + \\"color\\": \\"var(--mystery-color)\\" + } + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": \\"color:var(--mystery-color);display:block;margin:10px\\" + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": { + \\"margin-left\\": \\"Npx\\" + } + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 4, + \\"attributes\\": { + \\"style\\": { + \\"margin-top\\": \\"Npx\\", + \\"color\\": false + } + } + } + ], + \\"removes\\": [], + \\"adds\\": [] + } + } +]" +`; + exports[`record integration tests can use maskInputOptions to configure which type of inputs should be masked 1`] = ` "[ { diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index 39e32dbc..b61f76ec 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -209,6 +209,56 @@ describe('record integration tests', function (this: ISuite) { assertSnapshot(snapshots); }); + it('can record style changes compactly and preserve css var() functions', async () => { + const page: puppeteer.Page = await browser.newPage(); + await page.goto('about:blank'); + await page.setContent(getHtml.call(this, 'blank.html'), { + waitUntil: 'networkidle0', + }); + + // goal here is to ensure var(--mystery) ends up in the mutations (CSSOM fails in this case) + await page.evaluate( + 'document.body.setAttribute("style", "background: var(--mystery)")', + ); + await waitForRAF(page); + // and in this change we can't use the shorter styleObj format either + await page.evaluate( + 'document.body.setAttribute("style", "background: var(--mystery); background-color: black")', + ); + + // reset is always shorter to be recorded as a sting rather than a styleObj + await page.evaluate('document.body.setAttribute("style", "")'); + await waitForRAF(page); + + await page.evaluate('document.body.setAttribute("style", "display:block")'); + await waitForRAF(page); + // following should be recorded as an update of `{ color: 'var(--mystery-color)' }` without needing to include the display + await page.evaluate( + 'document.body.setAttribute("style", "color:var(--mystery-color);display:block")', + ); + await waitForRAF(page); + // whereas this case, it's shorter to record the entire string than the longhands for margin + await page.evaluate( + 'document.body.setAttribute("style", "color:var(--mystery-color);display:block;margin:10px")', + ); + await waitForRAF(page); + // and in this case, it's shorter to record just the change to the longhand margin-left; + await page.evaluate( + 'document.body.setAttribute("style", "color:var(--mystery-color);display:block;margin:10px 10px 10px 0px;")', + ); + await waitForRAF(page); + // see what happens when we manipulate the style object directly (expecting a compact mutation with just these two changes) + await page.evaluate( + 'document.body.style.marginTop = 0; document.body.style.color = null', + ); + await waitForRAF(page); + + const snapshots = (await page.evaluate( + 'window.snapshots', + )) as eventWithTime[]; + assertSnapshot(snapshots); + }); + it('can freeze mutations', async () => { const page: puppeteer.Page = await browser.newPage(); await page.goto('about:blank'); diff --git a/packages/rrweb/test/record/__snapshots__/cross-origin-iframes.test.ts.snap b/packages/rrweb/test/record/__snapshots__/cross-origin-iframes.test.ts.snap index 48d4bdb3..5caabe69 100644 --- a/packages/rrweb/test/record/__snapshots__/cross-origin-iframes.test.ts.snap +++ b/packages/rrweb/test/record/__snapshots__/cross-origin-iframes.test.ts.snap @@ -2625,10 +2625,7 @@ exports[`cross origin iframes form.html should map scroll events correctly 1`] = { \\"id\\": 9, \\"attributes\\": { - \\"style\\": { - \\"width\\": \\"Npx\\", - \\"height\\": \\"Npx\\" - } + \\"style\\": \\"width: Npx; height: Npx;\\" } } ], diff --git a/packages/rrweb/test/utils.ts b/packages/rrweb/test/utils.ts index dd5a8cf7..0d1e6400 100644 --- a/packages/rrweb/test/utils.ts +++ b/packages/rrweb/test/utils.ts @@ -133,23 +133,26 @@ function stringifySnapshots(snapshots: eventWithTime[]): string { s.data.source === IncrementalSource.Mutation ) { s.data.attributes.forEach((a) => { - if ( - 'style' in a.attributes && - a.attributes.style && - typeof a.attributes.style === 'object' - ) { - for (const [k, v] of Object.entries(a.attributes.style)) { - if (Array.isArray(v)) { - if (coordinatesReg.test(k + ': ' + v[0])) { - // TODO: could round the number here instead depending on what's coming out of various test envs - a.attributes.style[k] = ['Npx', v[1]]; - } - } else if (typeof v === 'string') { - if (coordinatesReg.test(k + ': ' + v)) { - a.attributes.style[k] = 'Npx'; + if ('style' in a.attributes && a.attributes.style) { + if (typeof a.attributes.style === 'object') { + for (const [k, v] of Object.entries(a.attributes.style)) { + if (Array.isArray(v)) { + if (coordinatesReg.test(k + ': ' + v[0])) { + // TODO: could round the number here instead depending on what's coming out of various test envs + a.attributes.style[k] = ['Npx', v[1]]; + } + } else if (typeof v === 'string') { + if (coordinatesReg.test(k + ': ' + v)) { + a.attributes.style[k] = 'Npx'; + } } + coordinatesReg.lastIndex = 0; // wow, a real wart in ECMAScript } - coordinatesReg.lastIndex = 0; // wow, a real wart in ECMAScript + } else if (coordinatesReg.test(a.attributes.style)) { + a.attributes.style = a.attributes.style.replace( + coordinatesReg, + '$1: Npx', + ); } } diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 66014572..e6f6f15c 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -283,7 +283,7 @@ export type textMutation = { value: string | null; }; -export type styleAttributeValue = { +export type styleOMValue = { [key: string]: styleValueWithPriority | string | false; }; @@ -292,13 +292,15 @@ export type styleValueWithPriority = [string, string]; export type attributeCursor = { node: Node; attributes: { - [key: string]: string | styleAttributeValue | null; + [key: string]: string | styleOMValue | null; }; + styleDiff: styleOMValue; + _unchangedStyles: styleOMValue; }; export type attributeMutation = { id: number; attributes: { - [key: string]: string | styleAttributeValue | null; + [key: string]: string | styleOMValue | null; }; };