Skip to content

Commit

Permalink
Realize we don't actually need to look at the .sheet during a mutat…
Browse files Browse the repository at this point in the history
…ion, even if there is only one text element on the <style>.

Recent conversation with Justin has firmed up my thinking on why we look at .sheet in the first place (it's not because we want to get rid of vendor prefixes, which may actually be undesirable where they are useful for accurate replay in the replayer - but rather it's to ensure we don't miss any programmatic style mutations that have happened - have added comments to reflect this)
  • Loading branch information
eoghanmurray committed May 7, 2024
1 parent 4cf6b73 commit 0c7403e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 20 deletions.
24 changes: 4 additions & 20 deletions packages/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,26 +557,10 @@ function serializeTextNode(
} else if (!blankTextNodes) {
textContent = n.textContent;
if (isStyle && textContent) {
// This branch is solely for the use of mutation
if (n.nextSibling || n.previousSibling) {
// This is not the only child of the stylesheet.
// We can't read all of the sheet's .cssRules and expect them
// to _only_ include the current rule(s) added by the text node.
// So we'll be conservative and keep textContent as-is.
} else if ((n.parentNode as HTMLStyleElement).sheet?.cssRules) {
try {
textContent = stringifyStylesheet(
(n.parentNode as HTMLStyleElement).sheet!,
);
} catch (err) {
console.warn(
`Cannot get CSS styles from text's parentNode. Error: ${
err as string
}`,
n,
);
}
}
// mutation only: we don't need to use stringifyStylesheet
// as a <style> text node mutation obliterates any previous
// programmatic rule manipulation (.insertRule etc.)
// so the current textContent represents the most up to date state
textContent = absoluteToStylesheet(textContent, getHref());
}
}
Expand Down
9 changes: 9 additions & 0 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ export function escapeImportStatement(rule: CSSImportRule): string {
return statement.join(' ') + ';';
}

/*
* serialize the css rules from the .sheet property
* for <link rel="stylesheet"> elements, this is the only way of getting the rules without a FETCH
* for <style> elements, this is less preferable to looking at childNodes[0].textContent
* (which will include vendor prefixed rules which may not be used or visible to the recorded browser,
* but which might be needed by the replayer browser)
* however, at snapshot time, we don't know whether the style element has suffered
* any programmatic manipulation prior to the snapshot, in which case the .sheet would be more up to date
*/
export function stringifyStylesheet(s: CSSStyleSheet): string | null {
try {
const rules = s.rules || s.cssRules;
Expand Down

0 comments on commit 0c7403e

Please sign in to comment.