Skip to content

Commit 40bbc25

Browse files
authored
fix: duplicate textContent for style element cause incremental style mutation invalid (#1417)
fix style element corner case - historically we have recorded duplicated css content in certain cases (demonstrated by the attached replayer test). This fix ensures that the replayer doesn't doubly add the content, which can cause problems when further mutations occur --------- Review and further tests contributed by: Eoghan Murray <eoghan@getthere.ie>
1 parent e0590bb commit 40bbc25

File tree

7 files changed

+610
-13
lines changed

7 files changed

+610
-13
lines changed

.changeset/cuddly-bikes-fail.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"rrweb-snapshot": patch
3+
"rrweb": patch
4+
---
5+
6+
fix: duplicate textContent for style elements cause incremental style mutations to be invalid

packages/rrweb-snapshot/src/rebuild.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,9 @@ function buildNode(
160160
value = adaptCssForReplay(value, cache);
161161
}
162162
if ((isTextarea || isRemoteOrDynamicCss) && typeof value === 'string') {
163-
node.appendChild(doc.createTextNode(value));
164163
// https://github.com/rrweb-io/rrweb/issues/112
164+
// https://github.com/rrweb-io/rrweb/pull/1351
165+
node.appendChild(doc.createTextNode(value));
165166
n.childNodes = []; // value overrides childNodes
166167
continue;
167168
}

packages/rrweb/src/replay/index.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,20 +1570,39 @@ export class Replayer {
15701570
if (
15711571
parentSn &&
15721572
parentSn.type === NodeType.Element &&
1573-
parentSn.tagName === 'textarea' &&
15741573
mutation.node.type === NodeType.Text
15751574
) {
1576-
const childNodeArray = Array.isArray(parent.childNodes)
1575+
const prospectiveSiblings = Array.isArray(parent.childNodes)
15771576
? parent.childNodes
15781577
: Array.from(parent.childNodes);
1579-
// This should be redundant now as we are either recording the value or the childNode, and not both
1580-
// keeping around for backwards compatibility with old bad double data, see
1581-
1582-
// https://github.com/rrweb-io/rrweb/issues/745
1583-
// parent is textarea, will only keep one child node as the value
1584-
for (const c of childNodeArray) {
1585-
if (c.nodeType === parent.TEXT_NODE) {
1586-
parent.removeChild(c as Node & RRNode);
1578+
if (parentSn.tagName === 'textarea') {
1579+
// This should be redundant now as we are either recording the value or the childNode, and not both
1580+
// keeping around for backwards compatibility with old bad double data, see
1581+
1582+
// https://github.com/rrweb-io/rrweb/issues/745
1583+
// parent is textarea, will only keep one child node as the value
1584+
for (const c of prospectiveSiblings) {
1585+
if (c.nodeType === parent.TEXT_NODE) {
1586+
parent.removeChild(c as Node & RRNode);
1587+
}
1588+
}
1589+
} else if (
1590+
parentSn.tagName === 'style' &&
1591+
prospectiveSiblings.length === 1
1592+
) {
1593+
// https://github.com/rrweb-io/rrweb/pull/1417
1594+
/**
1595+
* If both _cssText and textContent are present for a style element due to some existing bugs, the element was ending up with two child text nodes
1596+
* We need to remove the textNode created by _cssText as it doesn't have an id in the mirror, and thus cannot be further mutated.
1597+
*/
1598+
for (const cssText of prospectiveSiblings as (Node & RRNode)[]) {
1599+
if (
1600+
cssText.nodeType === parent.TEXT_NODE &&
1601+
!mirror.hasNode(cssText)
1602+
) {
1603+
target.textContent = cssText.textContent;
1604+
parent.removeChild(cssText);
1605+
}
15871606
}
15881607
}
15891608
} else if (parentSn?.type === NodeType.Document) {

packages/rrweb/test/__snapshots__/integration.test.ts.snap

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15851,6 +15851,224 @@ exports[`record integration tests > should record shadow doms polyfilled by synt
1585115851
]"
1585215852
`;
1585315853

15854+
exports[`record integration tests > should record style mutations and replay them correctly 1`] = `
15855+
"[
15856+
{
15857+
\\"type\\": 4,
15858+
\\"data\\": {
15859+
\\"href\\": \\"about:blank\\",
15860+
\\"width\\": 1920,
15861+
\\"height\\": 1080
15862+
}
15863+
},
15864+
{
15865+
\\"type\\": 2,
15866+
\\"data\\": {
15867+
\\"node\\": {
15868+
\\"type\\": 0,
15869+
\\"childNodes\\": [
15870+
{
15871+
\\"type\\": 1,
15872+
\\"name\\": \\"html\\",
15873+
\\"publicId\\": \\"\\",
15874+
\\"systemId\\": \\"\\",
15875+
\\"id\\": 2
15876+
},
15877+
{
15878+
\\"type\\": 2,
15879+
\\"tagName\\": \\"html\\",
15880+
\\"attributes\\": {
15881+
\\"lang\\": \\"en\\"
15882+
},
15883+
\\"childNodes\\": [
15884+
{
15885+
\\"type\\": 2,
15886+
\\"tagName\\": \\"head\\",
15887+
\\"attributes\\": {},
15888+
\\"childNodes\\": [
15889+
{
15890+
\\"type\\": 3,
15891+
\\"textContent\\": \\"\\\\n\\\\t \\",
15892+
\\"id\\": 5
15893+
},
15894+
{
15895+
\\"type\\": 2,
15896+
\\"tagName\\": \\"style\\",
15897+
\\"attributes\\": {
15898+
\\"_cssText\\": \\"#one { color: rgb(255, 0, 0); }\\"
15899+
},
15900+
\\"childNodes\\": [
15901+
{
15902+
\\"type\\": 3,
15903+
\\"textContent\\": \\"#one { color: rgb(255, 0, 0); }\\",
15904+
\\"isStyle\\": true,
15905+
\\"id\\": 7
15906+
}
15907+
],
15908+
\\"id\\": 6
15909+
},
15910+
{
15911+
\\"type\\": 3,
15912+
\\"textContent\\": \\"\\\\n \\",
15913+
\\"id\\": 8
15914+
},
15915+
{
15916+
\\"type\\": 2,
15917+
\\"tagName\\": \\"script\\",
15918+
\\"attributes\\": {},
15919+
\\"childNodes\\": [
15920+
{
15921+
\\"type\\": 3,
15922+
\\"textContent\\": \\"SCRIPT_PLACEHOLDER\\",
15923+
\\"id\\": 10
15924+
}
15925+
],
15926+
\\"id\\": 9
15927+
}
15928+
],
15929+
\\"id\\": 4
15930+
},
15931+
{
15932+
\\"type\\": 3,
15933+
\\"textContent\\": \\"\\\\n \\",
15934+
\\"id\\": 11
15935+
},
15936+
{
15937+
\\"type\\": 2,
15938+
\\"tagName\\": \\"body\\",
15939+
\\"attributes\\": {},
15940+
\\"childNodes\\": [
15941+
{
15942+
\\"type\\": 3,
15943+
\\"textContent\\": \\"\\\\n\\\\t \\",
15944+
\\"id\\": 13
15945+
},
15946+
{
15947+
\\"type\\": 2,
15948+
\\"tagName\\": \\"div\\",
15949+
\\"attributes\\": {
15950+
\\"id\\": \\"one\\"
15951+
},
15952+
\\"childNodes\\": [],
15953+
\\"id\\": 14
15954+
},
15955+
{
15956+
\\"type\\": 3,
15957+
\\"textContent\\": \\"\\\\n \\",
15958+
\\"id\\": 15
15959+
},
15960+
{
15961+
\\"type\\": 2,
15962+
\\"tagName\\": \\"div\\",
15963+
\\"attributes\\": {
15964+
\\"id\\": \\"two\\"
15965+
},
15966+
\\"childNodes\\": [],
15967+
\\"id\\": 16
15968+
},
15969+
{
15970+
\\"type\\": 3,
15971+
\\"textContent\\": \\"\\\\n\\\\t \\",
15972+
\\"id\\": 17
15973+
},
15974+
{
15975+
\\"type\\": 2,
15976+
\\"tagName\\": \\"script\\",
15977+
\\"attributes\\": {},
15978+
\\"childNodes\\": [
15979+
{
15980+
\\"type\\": 3,
15981+
\\"textContent\\": \\"SCRIPT_PLACEHOLDER\\",
15982+
\\"id\\": 19
15983+
}
15984+
],
15985+
\\"id\\": 18
15986+
},
15987+
{
15988+
\\"type\\": 3,
15989+
\\"textContent\\": \\"\\\\n \\\\n \\",
15990+
\\"id\\": 20
15991+
}
15992+
],
15993+
\\"id\\": 12
15994+
}
15995+
],
15996+
\\"id\\": 3
15997+
}
15998+
],
15999+
\\"id\\": 1
16000+
},
16001+
\\"initialOffset\\": {
16002+
\\"left\\": 0,
16003+
\\"top\\": 0
16004+
}
16005+
}
16006+
},
16007+
{
16008+
\\"type\\": 3,
16009+
\\"data\\": {
16010+
\\"source\\": 0,
16011+
\\"texts\\": [],
16012+
\\"attributes\\": [],
16013+
\\"removes\\": [],
16014+
\\"adds\\": [
16015+
{
16016+
\\"parentId\\": 4,
16017+
\\"nextId\\": null,
16018+
\\"node\\": {
16019+
\\"type\\": 2,
16020+
\\"tagName\\": \\"style\\",
16021+
\\"attributes\\": {
16022+
\\"_cssText\\": \\"#two { color: rgb(255, 0, 0); }\\"
16023+
},
16024+
\\"childNodes\\": [],
16025+
\\"id\\": 21
16026+
}
16027+
},
16028+
{
16029+
\\"parentId\\": 21,
16030+
\\"nextId\\": null,
16031+
\\"node\\": {
16032+
\\"type\\": 3,
16033+
\\"textContent\\": \\"#two { color: rgb(255, 0, 0); }\\",
16034+
\\"isStyle\\": true,
16035+
\\"id\\": 22
16036+
}
16037+
}
16038+
]
16039+
}
16040+
},
16041+
{
16042+
\\"type\\": 3,
16043+
\\"data\\": {
16044+
\\"source\\": 13,
16045+
\\"id\\": 6,
16046+
\\"set\\": {
16047+
\\"property\\": \\"color\\",
16048+
\\"value\\": \\"rgb(255, 255, 0)\\"
16049+
},
16050+
\\"index\\": [
16051+
0
16052+
]
16053+
}
16054+
},
16055+
{
16056+
\\"type\\": 3,
16057+
\\"data\\": {
16058+
\\"source\\": 13,
16059+
\\"id\\": 21,
16060+
\\"set\\": {
16061+
\\"property\\": \\"color\\",
16062+
\\"value\\": \\"rgb(255, 255, 0)\\"
16063+
},
16064+
\\"index\\": [
16065+
0
16066+
]
16067+
}
16068+
}
16069+
]"
16070+
`;
16071+
1585416072
exports[`record integration tests > should record webgl canvas mutations 1`] = `
1585516073
"[
1585616074
{

0 commit comments

Comments
 (0)