Skip to content

Commit

Permalink
Fix inline link elements bug (#995)
Browse files Browse the repository at this point in the history
* fix: bug when inlined link elements

* test: update snapshot for test cases

* apply Justin's review suggestions

1. make Mirror's replace function act the same with the original one when there's no existed node to get replaced.
2. when replacing with the link/style elements, keep their existing attributes to prevent potential bugs
  • Loading branch information
YunFeng0817 authored Sep 30, 2022
1 parent 3809060 commit 55ebce7
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 47 deletions.
5 changes: 5 additions & 0 deletions packages/rrdom/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,11 @@ export class Mirror implements IMirror<RRNode> {
}

replace(id: number, n: RRNode) {
const oldNode = this.getNode(id);
if (oldNode) {
const meta = this.nodeMetaMap.get(oldNode);
if (meta) this.nodeMetaMap.set(n, meta);
}
this.idNodeMap.set(id, n);
}

Expand Down
1 change: 0 additions & 1 deletion packages/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,6 @@ export function serializeNodeWithId(
},
stylesheetLoadTimeout,
);
if (isStylesheetLoaded(n as HTMLLinkElement) === false) return null; // add stylesheet in later mutation
}

return serializedNode;
Expand Down
2 changes: 1 addition & 1 deletion packages/rrweb/src/record/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ function record<T = eventWithTime>(
shadowDomManager.observeAttachShadow(iframe);
},
onStylesheetLoad: (linkEl, childSn) => {
stylesheetManager.attachLinkElement(linkEl, childSn, mirror);
stylesheetManager.attachLinkElement(linkEl, childSn);
},
keepIframeSrcFn,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/rrweb/src/record/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export default class MutationBuffer {
this.shadowDomManager.observeAttachShadow(iframe);
},
onStylesheetLoad: (link, childSn) => {
this.stylesheetManager.attachLinkElement(link, childSn, this.mirror);
this.stylesheetManager.attachLinkElement(link, childSn);
},
});
if (sn) {
Expand Down
29 changes: 15 additions & 14 deletions packages/rrweb/src/record/stylesheet-manager.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { Mirror, serializedNodeWithId } from 'rrweb-snapshot';
import type { elementNode, serializedNodeWithId } from 'rrweb-snapshot';
import { getCssRuleString } from 'rrweb-snapshot';
import type {
adoptedStyleSheetCallback,
adoptedStyleSheetParam,
attributeMutation,
mutationCallBack,
} from '../types';
import { StyleSheetMirror } from '../utils';
Expand All @@ -24,20 +25,20 @@ export class StylesheetManager {
public attachLinkElement(
linkEl: HTMLLinkElement,
childSn: serializedNodeWithId,
mirror: Mirror,
) {
this.mutationCb({
adds: [
{
parentId: mirror.getId(linkEl),
nextId: null,
node: childSn,
},
],
removes: [],
texts: [],
attributes: [],
});
if ('_cssText' in (childSn as elementNode).attributes)
this.mutationCb({
adds: [],
removes: [],
texts: [],
attributes: [
{
id: childSn.id,
attributes: (childSn as elementNode)
.attributes as attributeMutation['attributes'],
},
],
});

this.trackLinkElement(linkEl);
}
Expand Down
37 changes: 37 additions & 0 deletions packages/rrweb/src/replay/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
createCache,
Mirror,
createMirror,
attributes,
serializedElementNodeWithId,
} from 'rrweb-snapshot';
import {
RRDocument,
Expand Down Expand Up @@ -1643,6 +1645,41 @@ export class Replayer {
(target as Element | RRElement).removeAttribute(attributeName);
} else if (typeof value === 'string') {
try {
// When building snapshot, some link styles haven't loaded. Then they are loaded, they will be inlined as incremental mutation change of attribute. We need to replace the old elements whose styles aren't inlined.
if (
attributeName === '_cssText' &&
(target.nodeName === 'LINK' || target.nodeName === 'STYLE')
) {
try {
const newSn = mirror.getMeta(
target as Node & RRNode,
) as serializedElementNodeWithId;
Object.assign(
newSn.attributes,
mutation.attributes as attributes,
);
const newNode = buildNodeWithSN(newSn, {
doc: target.ownerDocument as Document, // can be Document or RRDocument
mirror: mirror as Mirror,
skipChild: true,
hackCss: true,
cache: this.cache,
});
const siblingNode = target.nextSibling;
const parentNode = target.parentNode;
if (newNode && parentNode) {
parentNode.removeChild(target as Node & RRNode);
parentNode.insertBefore(
newNode as Node & RRNode,
siblingNode as (Node & RRNode) | null,
);
mirror.replace(mutation.id, newNode as Node & RRNode);
break;
}
} catch (e) {
// for safe
}
}
(target as Element | RRElement).setAttribute(
attributeName,
value,
Expand Down
74 changes: 48 additions & 26 deletions packages/rrweb/test/__snapshots__/record.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,12 @@ exports[`record captures CORS stylesheets that are still loading 1`] = `
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [],
\\"removes\\": [],
\\"adds\\": [
{
\\"parentId\\": 9,
\\"parentId\\": 4,
\\"nextId\\": null,
\\"node\\": {
\\"type\\": 2,
Expand All @@ -188,10 +191,7 @@ exports[`record captures CORS stylesheets that are still loading 1`] = `
\\"id\\": 9
}
}
],
\\"removes\\": [],
\\"texts\\": [],
\\"attributes\\": []
]
}
}
]"
Expand Down Expand Up @@ -2007,7 +2007,19 @@ exports[`record captures stylesheets in iframes that are still loading 1`] = `
\\"type\\": 2,
\\"tagName\\": \\"head\\",
\\"attributes\\": {},
\\"childNodes\\": [],
\\"childNodes\\": [
{
\\"type\\": 2,
\\"tagName\\": \\"link\\",
\\"attributes\\": {
\\"rel\\": \\"stylesheet\\",
\\"href\\": \\"blob:null\\"
},
\\"childNodes\\": [],
\\"rootId\\": 10,
\\"id\\": 13
}
],
\\"rootId\\": 10,
\\"id\\": 12
},
Expand Down Expand Up @@ -2039,25 +2051,17 @@ exports[`record captures stylesheets in iframes that are still loading 1`] = `
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"adds\\": [
\\"adds\\": [],
\\"removes\\": [],
\\"texts\\": [],
\\"attributes\\": [
{
\\"parentId\\": 13,
\\"nextId\\": null,
\\"node\\": {
\\"type\\": 2,
\\"tagName\\": \\"link\\",
\\"attributes\\": {
\\"_cssText\\": \\"body { color: pink; }\\"
},
\\"childNodes\\": [],
\\"rootId\\": 10,
\\"id\\": 13
\\"id\\": 13,
\\"attributes\\": {
\\"_cssText\\": \\"body { color: pink; }\\"
}
}
],
\\"removes\\": [],
\\"texts\\": [],
\\"attributes\\": []
]
}
}
]"
Expand Down Expand Up @@ -2286,24 +2290,42 @@ exports[`record captures stylesheets that are still loading 1`] = `
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [],
\\"removes\\": [],
\\"adds\\": [
{
\\"parentId\\": 9,
\\"parentId\\": 4,
\\"nextId\\": null,
\\"node\\": {
\\"type\\": 2,
\\"tagName\\": \\"link\\",
\\"attributes\\": {
\\"_cssText\\": \\"body { color: pink; }\\"
\\"rel\\": \\"stylesheet\\",
\\"href\\": \\"blob:null\\"
},
\\"childNodes\\": [],
\\"id\\": 9
}
}
],
]
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"adds\\": [],
\\"removes\\": [],
\\"texts\\": [],
\\"attributes\\": []
\\"attributes\\": [
{
\\"id\\": 9,
\\"attributes\\": {
\\"_cssText\\": \\"body { color: pink; }\\"
}
}
]
}
}
]"
Expand Down
13 changes: 9 additions & 4 deletions packages/rrweb/test/record.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,11 @@ describe('record', function (this: ISuite) {

// `blob:` URLs are not available immediately, so we need to wait for the browser to load them
await waitForRAF(ctx.page);

assertSnapshot(ctx.events);
// 'blob' URL is different in every execution so we need to remove it from the snapshot.
const filteredEvents = JSON.parse(
JSON.stringify(ctx.events).replace(/blob\:[\w\d-/]+"/, 'blob:null"'),
);
assertSnapshot(filteredEvents);
});

it('captures stylesheets in iframes that are still loading', async () => {
Expand Down Expand Up @@ -659,8 +662,10 @@ describe('record', function (this: ISuite) {

// `blob:` URLs are not available immediately, so we need to wait for the browser to load them
await waitForRAF(ctx.page);

assertSnapshot(ctx.events);
const filteredEvents = JSON.parse(
JSON.stringify(ctx.events).replace(/blob\:[\w\d-/]+"/, 'blob:null"'),
);
assertSnapshot(filteredEvents);
});

it('captures CORS stylesheets that are still loading', async () => {
Expand Down

0 comments on commit 55ebce7

Please sign in to comment.