Skip to content

Commit 8e47ca1

Browse files
mydeamydea
authored
fix: Explicitly handle null attribute values (#1157)
* fix: Explicitly handle removed attributes The attribute `value` can be null when a mutation observer triggers due to a removed attribute. This is currently not reflected by types and code. * Apply formatting changes * fix * add changeset
1 parent e65465e commit 8e47ca1

7 files changed

Lines changed: 256 additions & 15 deletions

File tree

‎.changeset/swift-peas-film.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: Explicitly handle `null` attribute values

‎packages/rrweb-snapshot/src/rebuild.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ function buildNode(
149149
* They often overwrite other attributes on the element.
150150
* We need to parse them last so they can overwrite conflicting attributes.
151151
*/
152-
const specialAttributes: attributes = {};
152+
const specialAttributes: { [key: string]: string | number } = {};
153153
for (const name in n.attributes) {
154154
if (!Object.prototype.hasOwnProperty.call(n.attributes, name)) {
155155
continue;
@@ -165,6 +165,11 @@ function buildNode(
165165
continue;
166166
}
167167

168+
// null values mean the attribute was removed
169+
if (value === null) {
170+
continue;
171+
}
172+
168173
/**
169174
* Boolean attributes are considered to be true if they're present on the element at all.
170175
* We should set value to the empty string ("") or the attribute's name, with no leading or trailing whitespace.

‎packages/rrweb-snapshot/src/snapshot.ts‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,33 +223,36 @@ export function transformAttribute(
223223
doc: Document,
224224
tagName: string,
225225
name: string,
226-
value: string,
227-
): string {
226+
value: string | null,
227+
): string | null {
228+
if (!value) {
229+
return value;
230+
}
231+
228232
// relative path in attribute
229233
if (
230234
name === 'src' ||
231-
(name === 'href' && value && !(tagName === 'use' && value[0] === '#'))
235+
(name === 'href' && !(tagName === 'use' && value[0] === '#'))
232236
) {
233237
// href starts with a # is an id pointer for svg
234238
return absoluteToDoc(doc, value);
235-
} else if (name === 'xlink:href' && value && value[0] !== '#') {
239+
} else if (name === 'xlink:href' && value[0] !== '#') {
236240
// xlink:href starts with # is an id pointer
237241
return absoluteToDoc(doc, value);
238242
} else if (
239243
name === 'background' &&
240-
value &&
241244
(tagName === 'table' || tagName === 'td' || tagName === 'th')
242245
) {
243246
return absoluteToDoc(doc, value);
244-
} else if (name === 'srcset' && value) {
247+
} else if (name === 'srcset') {
245248
return getAbsoluteSrcsetString(doc, value);
246-
} else if (name === 'style' && value) {
249+
} else if (name === 'style') {
247250
return absoluteToStylesheet(value, getHref());
248-
} else if (tagName === 'object' && name === 'data' && value) {
251+
} else if (tagName === 'object' && name === 'data') {
249252
return absoluteToDoc(doc, value);
250-
} else {
251-
return value;
252253
}
254+
255+
return value;
253256
}
254257

255258
export function _isBlockedElement(
@@ -794,8 +797,10 @@ function serializeElementNode(
794797
};
795798
}
796799

797-
function lowerIfExists(maybeAttr: string | number | boolean): string {
798-
if (maybeAttr === undefined) {
800+
function lowerIfExists(
801+
maybeAttr: string | number | boolean | undefined | null,
802+
): string {
803+
if (maybeAttr === undefined || maybeAttr === null) {
799804
return '';
800805
} else {
801806
return (maybeAttr as string).toLowerCase();

‎packages/rrweb-snapshot/src/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export type documentTypeNode = {
2121
};
2222

2323
export type attributes = {
24-
[key: string]: string | number | true;
24+
[key: string]: string | number | true | null;
2525
};
2626
export type legacyAttributes = {
2727
/**

‎packages/rrweb/src/record/mutation.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ export default class MutationBuffer {
563563
this.doc,
564564
target.tagName,
565565
m.attributeName!,
566-
value!,
566+
value,
567567
);
568568
}
569569
break;

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

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3960,6 +3960,205 @@ exports[`record integration tests can use maskInputOptions to configure which ty
39603960
]"
39613961
`;
39623962

3963+
exports[`record integration tests handles null attribute values 1`] = `
3964+
"[
3965+
{
3966+
\\"type\\": 0,
3967+
\\"data\\": {}
3968+
},
3969+
{
3970+
\\"type\\": 1,
3971+
\\"data\\": {}
3972+
},
3973+
{
3974+
\\"type\\": 4,
3975+
\\"data\\": {
3976+
\\"href\\": \\"about:blank\\",
3977+
\\"width\\": 1920,
3978+
\\"height\\": 1080
3979+
}
3980+
},
3981+
{
3982+
\\"type\\": 2,
3983+
\\"data\\": {
3984+
\\"node\\": {
3985+
\\"type\\": 0,
3986+
\\"childNodes\\": [
3987+
{
3988+
\\"type\\": 1,
3989+
\\"name\\": \\"html\\",
3990+
\\"publicId\\": \\"\\",
3991+
\\"systemId\\": \\"\\",
3992+
\\"id\\": 2
3993+
},
3994+
{
3995+
\\"type\\": 2,
3996+
\\"tagName\\": \\"html\\",
3997+
\\"attributes\\": {},
3998+
\\"childNodes\\": [
3999+
{
4000+
\\"type\\": 2,
4001+
\\"tagName\\": \\"head\\",
4002+
\\"attributes\\": {},
4003+
\\"childNodes\\": [],
4004+
\\"id\\": 4
4005+
},
4006+
{
4007+
\\"type\\": 2,
4008+
\\"tagName\\": \\"body\\",
4009+
\\"attributes\\": {},
4010+
\\"childNodes\\": [
4011+
{
4012+
\\"type\\": 3,
4013+
\\"textContent\\": \\"\\\\n \\",
4014+
\\"id\\": 6
4015+
},
4016+
{
4017+
\\"type\\": 2,
4018+
\\"tagName\\": \\"p\\",
4019+
\\"attributes\\": {},
4020+
\\"childNodes\\": [
4021+
{
4022+
\\"type\\": 3,
4023+
\\"textContent\\": \\"mutation observer\\",
4024+
\\"id\\": 8
4025+
}
4026+
],
4027+
\\"id\\": 7
4028+
},
4029+
{
4030+
\\"type\\": 3,
4031+
\\"textContent\\": \\"\\\\n \\",
4032+
\\"id\\": 9
4033+
},
4034+
{
4035+
\\"type\\": 2,
4036+
\\"tagName\\": \\"ul\\",
4037+
\\"attributes\\": {},
4038+
\\"childNodes\\": [
4039+
{
4040+
\\"type\\": 3,
4041+
\\"textContent\\": \\"\\\\n \\",
4042+
\\"id\\": 11
4043+
},
4044+
{
4045+
\\"type\\": 2,
4046+
\\"tagName\\": \\"li\\",
4047+
\\"attributes\\": {},
4048+
\\"childNodes\\": [],
4049+
\\"id\\": 12
4050+
},
4051+
{
4052+
\\"type\\": 3,
4053+
\\"textContent\\": \\"\\\\n \\",
4054+
\\"id\\": 13
4055+
}
4056+
],
4057+
\\"id\\": 10
4058+
},
4059+
{
4060+
\\"type\\": 3,
4061+
\\"textContent\\": \\"\\\\n \\",
4062+
\\"id\\": 14
4063+
},
4064+
{
4065+
\\"type\\": 2,
4066+
\\"tagName\\": \\"canvas\\",
4067+
\\"attributes\\": {},
4068+
\\"childNodes\\": [],
4069+
\\"id\\": 15
4070+
},
4071+
{
4072+
\\"type\\": 3,
4073+
\\"textContent\\": \\"\\\\n\\\\n \\",
4074+
\\"id\\": 16
4075+
},
4076+
{
4077+
\\"type\\": 2,
4078+
\\"tagName\\": \\"script\\",
4079+
\\"attributes\\": {},
4080+
\\"childNodes\\": [
4081+
{
4082+
\\"type\\": 3,
4083+
\\"textContent\\": \\"SCRIPT_PLACEHOLDER\\",
4084+
\\"id\\": 18
4085+
}
4086+
],
4087+
\\"id\\": 17
4088+
},
4089+
{
4090+
\\"type\\": 3,
4091+
\\"textContent\\": \\"\\\\n \\\\n \\\\n\\",
4092+
\\"id\\": 19
4093+
}
4094+
],
4095+
\\"id\\": 5
4096+
}
4097+
],
4098+
\\"id\\": 3
4099+
}
4100+
],
4101+
\\"id\\": 1
4102+
},
4103+
\\"initialOffset\\": {
4104+
\\"left\\": 0,
4105+
\\"top\\": 0
4106+
}
4107+
}
4108+
},
4109+
{
4110+
\\"type\\": 3,
4111+
\\"data\\": {
4112+
\\"source\\": 0,
4113+
\\"texts\\": [],
4114+
\\"attributes\\": [
4115+
{
4116+
\\"id\\": 20,
4117+
\\"attributes\\": {
4118+
\\"aria-label\\": \\"label\\",
4119+
\\"id\\": \\"test-li\\"
4120+
}
4121+
}
4122+
],
4123+
\\"removes\\": [],
4124+
\\"adds\\": [
4125+
{
4126+
\\"parentId\\": 10,
4127+
\\"nextId\\": null,
4128+
\\"node\\": {
4129+
\\"type\\": 2,
4130+
\\"tagName\\": \\"li\\",
4131+
\\"attributes\\": {
4132+
\\"aria-label\\": \\"label\\",
4133+
\\"id\\": \\"test-li\\"
4134+
},
4135+
\\"childNodes\\": [],
4136+
\\"id\\": 20
4137+
}
4138+
}
4139+
]
4140+
}
4141+
},
4142+
{
4143+
\\"type\\": 3,
4144+
\\"data\\": {
4145+
\\"source\\": 0,
4146+
\\"texts\\": [],
4147+
\\"attributes\\": [
4148+
{
4149+
\\"id\\": 20,
4150+
\\"attributes\\": {
4151+
\\"aria-label\\": null
4152+
}
4153+
}
4154+
],
4155+
\\"removes\\": [],
4156+
\\"adds\\": []
4157+
}
4158+
}
4159+
]"
4160+
`;
4161+
39634162
exports[`record integration tests mutations should work when blocked class is unblocked 1`] = `
39644163
"[
39654164
{

‎packages/rrweb/test/integration.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,32 @@ describe('record integration tests', function (this: ISuite) {
141141
assertSnapshot(snapshots);
142142
});
143143

144+
it('handles null attribute values', async () => {
145+
const page: puppeteer.Page = await browser.newPage();
146+
await page.goto('about:blank');
147+
await page.setContent(getHtml.call(this, 'mutation-observer.html', {}));
148+
149+
await page.evaluate(() => {
150+
const li = document.createElement('li');
151+
const ul = document.querySelector('ul') as HTMLUListElement;
152+
ul.appendChild(li);
153+
154+
li.setAttribute('aria-label', 'label');
155+
li.setAttribute('id', 'test-li');
156+
});
157+
158+
await new Promise((resolve) => setTimeout(resolve, 100));
159+
160+
await page.evaluate(() => {
161+
const li = document.querySelector('#test-li') as HTMLLIElement;
162+
// This triggers the mutation observer with a `null` attribute value
163+
li.removeAttribute('aria-label');
164+
});
165+
166+
const snapshots = await page.evaluate('window.snapshots');
167+
assertSnapshot(snapshots);
168+
});
169+
144170
it('can record node mutations', async () => {
145171
const page: puppeteer.Page = await browser.newPage();
146172
await page.goto('about:blank');

0 commit comments

Comments
 (0)