Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inline link elements bug #995

Merged
merged 3 commits into from
Sep 30, 2022
Merged

Fix inline link elements bug #995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
406a7fb
fix: bug when inlined link elements
YunFeng0817 Sep 23, 2022
10a750d
test: update snapshot for test cases
YunFeng0817 Sep 23, 2022
8aefe2b
apply Justin's review suggestions
YunFeng0817 Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/rrdom/src/index.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is _cssText always going to be set, also if a link element hasn't finished loading yet?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According the code below

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

Lines 653 to 659 in 08ab7ab

if (stylesheet) {
cssText = getCssRulesString(stylesheet);
}
if (cssText) {
delete attributes.rel;
delete attributes.href;
attributes._cssText = absoluteToStylesheet(cssText, stylesheet!.href!);

I think it will set _cssText when the link has been loaded. Otherwise, recorder will only keep its original 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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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