Skip to content

Commit

Permalink
Add workaround for Chrome/Edge css import escaping bug (#1287)
Browse files Browse the repository at this point in the history
* Upgrade to typescript 4.9.5

* Apply formatting changes

* Add workaround for chrome incorrect escaping bug

More info: https://bugs.chromium.org/p/chromium/issues/detail?id=1472259

* Apply formatting changes

* Create itchy-dryers-double.md

* Create rich-jars-remember.md

* Apply formatting changes

* Update packages/rrweb-snapshot/src/css.ts

* Apply formatting changes

* Update packages/rrweb-snapshot/test/__snapshots__/integration.test.ts.snap

* Apply formatting changes

* Update snapshot

* Apply formatting changes

* Rename and refactor fixBrowserCompatibilityIssuesInCSSImports, getCssRulesString and getCssRuleString based on @eoghanmurray feedback

* Apply formatting changes

* Apply formatting changes
  • Loading branch information
Juice10 authored Aug 11, 2023
1 parent 11f6567 commit efdc167
Show file tree
Hide file tree
Showing 18 changed files with 343 additions and 156 deletions.
8 changes: 8 additions & 0 deletions .changeset/itchy-dryers-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'rrweb-player': patch
'rrweb-snapshot': patch
'rrweb': patch
'@rrweb/types': patch
---

Upgrade all projects to typescript 4.9.5
5 changes: 5 additions & 0 deletions .changeset/rich-jars-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'rrweb-snapshot': patch
---

Add workaround for Chrome/Edge CSS `@import` escaping bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1472259
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"markdownlint-cli": "^0.31.1",
"prettier": "2.8.4",
"turbo": "^1.2.4",
"typescript": "^4.7.3"
"typescript": "^4.9.5"
},
"scripts": {
"build:all": "NODE_OPTIONS='--max-old-space-size=4096' yarn run concurrently --success=all -r -m=1 'yarn workspaces-to-typescript-project-references' 'yarn turbo run prepublish'",
Expand All @@ -49,7 +49,8 @@
"release": "yarn build:all && changeset publish"
},
"resolutions": {
"**/jsdom/cssom": "https://registry.npmjs.org/rrweb-cssom/-/rrweb-cssom-0.6.0.tgz"
"**/jsdom/cssom": "https://registry.npmjs.org/rrweb-cssom/-/rrweb-cssom-0.6.0.tgz",
"**/@types/dom-webcodecs": "0.1.5"
},
"browserslist": [
"defaults",
Expand Down
6 changes: 3 additions & 3 deletions packages/rrweb-player/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
"rollup-plugin-typescript2": "^0.31.2",
"rollup-plugin-web-worker-loader": "^1.6.1",
"sirv-cli": "^0.4.4",
"svelte": "^3.2.0",
"svelte-check": "^1.4.0",
"svelte-preprocess": "^4.0.0",
"svelte": "^3.59.2",
"svelte-check": "^3.0.1",
"svelte-preprocess": "^5.0.0",
"tslib": "^2.0.0"
},
"dependencies": {
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb-snapshot/src/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ export function parse(css: string, options: ParserOptions = {}) {
whitespace();
comments(rules);
while (css.length && css.charAt(0) !== '}' && (node = atrule() || rule())) {
if (node !== false) {
if (node) {
rules.push(node);
comments(rules);
}
Expand Down Expand Up @@ -383,7 +383,7 @@ export function parse(css: string, options: ParserOptions = {}) {
function comments(rules: Rule[] = []) {
let c: Comment | void;
while ((c = comment())) {
if (c !== false) {
if (c) {
rules.push(c);
}
c = comment();
Expand Down
8 changes: 4 additions & 4 deletions packages/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
isShadowRoot,
maskInputValue,
isNativeShadowDom,
getCssRulesString,
stringifyStylesheet,
getInputType,
toLowerCase,
validateStringifiedCssRule,
Expand Down Expand Up @@ -554,7 +554,7 @@ function serializeTextNode(
// 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) {
textContent = getCssRulesString(
textContent = stringifyStylesheet(
(n.parentNode as HTMLStyleElement).sheet!,
);
}
Expand Down Expand Up @@ -644,7 +644,7 @@ function serializeElementNode(
});
let cssText: string | null = null;
if (stylesheet) {
cssText = getCssRulesString(stylesheet);
cssText = stringifyStylesheet(stylesheet);
}
if (cssText) {
delete attributes.rel;
Expand All @@ -659,7 +659,7 @@ function serializeElementNode(
// TODO: Currently we only try to get dynamic stylesheet when it is an empty style element
!(n.innerText || n.textContent || '').trim().length
) {
const cssText = getCssRulesString(
const cssText = stringifyStylesheet(
(n as HTMLStyleElement).sheet as CSSStyleSheet,
);
if (cssText) {
Expand Down
59 changes: 52 additions & 7 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,74 @@ function fixBrowserCompatibilityIssuesInCSS(cssText: string): string {
return cssText;
}

export function getCssRulesString(s: CSSStyleSheet): string | null {
// Remove this declaration once typescript has added `CSSImportRule.supportsText` to the lib.
declare interface CSSImportRule extends CSSRule {
readonly href: string;
readonly layerName: string | null;
readonly media: MediaList;
readonly styleSheet: CSSStyleSheet;
/**
* experimental API, currently only supported in firefox
* https://developer.mozilla.org/en-US/docs/Web/API/CSSImportRule/supportsText
*/
readonly supportsText?: string | null;
}

/**
* Browsers sometimes incorrectly escape `@import` on `.cssText` statements.
* This function tries to correct the escaping.
* more info: https://bugs.chromium.org/p/chromium/issues/detail?id=1472259
* @param cssImportRule
* @returns `cssText` with browser inconsistencies fixed, or null if not applicable.
*/
export function escapeImportStatement(rule: CSSImportRule): string {
const { cssText } = rule;
if (cssText.split('"').length < 3) return cssText;

const statement = ['@import', `url(${JSON.stringify(rule.href)})`];
if (rule.layerName === '') {
statement.push(`layer`);
} else if (rule.layerName) {
statement.push(`layer(${rule.layerName})`);
}
if (rule.supportsText) {
statement.push(`supports(${rule.supportsText})`);
}
if (rule.media.length) {
statement.push(rule.media.mediaText);
}
return statement.join(' ') + ';';
}

export function stringifyStylesheet(s: CSSStyleSheet): string | null {
try {
const rules = s.rules || s.cssRules;
return rules
? fixBrowserCompatibilityIssuesInCSS(
Array.from(rules).map(getCssRuleString).join(''),
Array.from(rules).map(stringifyRule).join(''),
)
: null;
} catch (error) {
return null;
}
}

export function getCssRuleString(rule: CSSRule): string {
let cssStringified = rule.cssText;
export function stringifyRule(rule: CSSRule): string {
let importStringified;
if (isCSSImportRule(rule)) {
try {
cssStringified = getCssRulesString(rule.styleSheet) || cssStringified;
} catch {
importStringified =
// for same-origin stylesheets,
// we can access the imported stylesheet rules directly
stringifyStylesheet(rule.styleSheet) ||
// work around browser issues with the raw string `@import url(...)` statement
escapeImportStatement(rule);
} catch (error) {
// ignore
}
}
return validateStringifiedCssRule(cssStringified);

return validateStringifiedCssRule(importStringified || rule.cssText);
}

export function validateStringifiedCssRule(cssStringified: string): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ exports[`integration tests [html file]: with-style-sheet-with-import.html 1`] =
<meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1.0\\" />
<meta http-equiv=\\"X-UA-Compatible\\" content=\\"ie=edge\\" />
<title>with style sheet with import</title>
<style>body { margin: 0px; background: url(\\"http://localhost:3030/a.jpg\\"); border-image: url(\\"data:image/svg+xml;utf8,&lt;svg xmlns=\\\\\\"http://www.w3.org/2000/svg\\\\\\" x=\\\\\\"0px\\\\\\" y=\\\\\\"0px\\\\\\" viewBox=\\\\\\"0 0 256 256\\\\\\"&gt;&lt;g&gt;&lt;g&gt;&lt;polygon points=\\\\\\"79.093,0 48.907,30.187 146.72,128 48.907,225.813 79.093,256 207.093,128\\\\\\"/&gt;&lt;/g&gt;&lt;/g&gt;&lt;/svg&gt;\\") 100% / 1 / 0 stretch; }p { color: red; background: url(\\"http://localhost:3030/css/b.jpg\\"); }body &gt; p { color: yellow; }</style>
<style>@import url(\\"//fonts.googleapis.com/css2?family=Open+Sans:wght@400;600;700&amp;family=Roboto:wght@100;300;400;500;700&amp;display=swap\\\\\\"\\");body { margin: 0px; background: url(\\"http://localhost:3030/a.jpg\\"); border-image: url(\\"data:image/svg+xml;utf8,&lt;svg xmlns=\\\\\\"http://www.w3.org/2000/svg\\\\\\" x=\\\\\\"0px\\\\\\" y=\\\\\\"0px\\\\\\" viewBox=\\\\\\"0 0 256 256\\\\\\"&gt;&lt;g&gt;&lt;g&gt;&lt;polygon points=\\\\\\"79.093,0 48.907,30.187 146.72,128 48.907,225.813 79.093,256 207.093,128\\\\\\"/&gt;&lt;/g&gt;&lt;/g&gt;&lt;/svg&gt;\\") 100% / 1 / 0 stretch; }p { color: red; background: url(\\"http://localhost:3030/css/b.jpg\\"); }body &gt; p { color: yellow; }</style>
</head><body>
</body></html>"
`;
Expand Down
65 changes: 64 additions & 1 deletion packages/rrweb-snapshot/test/css.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { parse, Rule, Media } from '../src/css';
import { validateStringifiedCssRule } from './../src/utils';
import {
validateStringifiedCssRule,
escapeImportStatement,
} from './../src/utils';

describe('css parser', () => {
it('should save the filename and source', () => {
Expand Down Expand Up @@ -120,4 +123,64 @@ describe('css parser', () => {
);
expect(out3).toEqual('[data-aa\\:other] { color: red; }');
});

it('parses imports with quotes correctly', () => {
const out1 = escapeImportStatement({
cssText: `@import url("/foo.css;900;800"");`,
href: '/foo.css;900;800"',
media: {
length: 0,
},
layerName: null,
supportsText: null,
} as unknown as CSSImportRule);
expect(out1).toEqual(`@import url("/foo.css;900;800\\"");`);

const out2 = escapeImportStatement({
cssText: `@import url("/foo.css;900;800"") supports(display: flex);`,
href: '/foo.css;900;800"',
media: {
length: 0,
},
layerName: null,
supportsText: 'display: flex',
} as unknown as CSSImportRule);
expect(out2).toEqual(
`@import url("/foo.css;900;800\\"") supports(display: flex);`,
);

const out3 = escapeImportStatement({
cssText: `@import url("/foo.css;900;800"");`,
href: '/foo.css;900;800"',
media: {
length: 1,
mediaText: 'print, screen',
},
layerName: null,
supportsText: null,
} as unknown as CSSImportRule);
expect(out3).toEqual(`@import url("/foo.css;900;800\\"") print, screen;`);

const out4 = escapeImportStatement({
cssText: `@import url("/foo.css;900;800"") layer(layer-1);`,
href: '/foo.css;900;800"',
media: {
length: 0,
},
layerName: 'layer-1',
supportsText: null,
} as unknown as CSSImportRule);
expect(out4).toEqual(`@import url("/foo.css;900;800\\"") layer(layer-1);`);

const out5 = escapeImportStatement({
cssText: `@import url("/foo.css;900;800"") layer;`,
href: '/foo.css;900;800"',
media: {
length: 0,
},
layerName: '',
supportsText: null,
} as unknown as CSSImportRule);
expect(out5).toEqual(`@import url("/foo.css;900;800\\"") layer;`);
});
});
3 changes: 2 additions & 1 deletion packages/rrweb-snapshot/test/css/style-with-import.css
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
@import "./style.css";
@import '//fonts.googleapis.com/css2?family=Open+Sans:wght@400;600;700&family=Roboto:wght@100;300;400;500;700&display=swap"';
@import './style.css';
2 changes: 1 addition & 1 deletion packages/rrweb/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"devDependencies": {
"@rollup/plugin-node-resolve": "^13.1.3",
"@types/chai": "^4.1.6",
"@types/dom-mediacapture-transform": "^0.1.3",
"@types/dom-mediacapture-transform": "0.1.4",
"@types/inquirer": "^8.2.1",
"@types/jest": "^29.5.0",
"@types/jest-image-snapshot": "^6.1.0",
Expand Down
10 changes: 0 additions & 10 deletions packages/rrweb/src/record/constructable-stylesheets.d.ts

This file was deleted.

6 changes: 3 additions & 3 deletions packages/rrweb/src/record/observers/canvas/canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@ export default function initCanvasContextObserver(
'getContext',
function (
original: (
this: ICanvas,
this: ICanvas | HTMLCanvasElement,
contextType: string,
...args: Array<unknown>
) => void,
) {
return function (
this: ICanvas,
this: ICanvas | HTMLCanvasElement,
contextType: string,
...args: Array<unknown>
) {
if (!isBlocked(this, blockClass, blockSelector, true)) {
const ctxName = getNormalizedContextName(contextType);
if (!('__context' in this)) this.__context = ctxName;
if (!('__context' in this)) (this as ICanvas).__context = ctxName;

if (
setPreserveDrawingBufferToTrue &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export function serializeArg(
};
}

return value as CanvasArg;
return value as unknown as CanvasArg;
}

export const serializeArgs = (
Expand Down
5 changes: 4 additions & 1 deletion packages/rrweb/src/record/observers/canvas/webgl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ function patchGLPrototype(
return function (this: typeof prototype, ...args: Array<unknown>) {
const result = original.apply(this, args);
saveWebGLVar(result, win, this);
if (!isBlocked(this.canvas, blockClass, blockSelector, true)) {
if (
'tagName' in this.canvas &&
!isBlocked(this.canvas, blockClass, blockSelector, true)
) {
const recordArgs = serializeArgs([...args], win, this);
const mutation: canvasMutationWithType = {
type,
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb/src/record/stylesheet-manager.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { elementNode, serializedNodeWithId } from 'rrweb-snapshot';
import { getCssRuleString } from 'rrweb-snapshot';
import { stringifyRule } from 'rrweb-snapshot';
import type {
adoptedStyleSheetCallback,
adoptedStyleSheetParam,
Expand Down Expand Up @@ -66,7 +66,7 @@ export class StylesheetManager {
styleId,
rules: rules.map((r, index) => {
return {
rule: getCssRuleString(r),
rule: stringifyRule(r),
index,
};
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
],
"devDependencies": {
"vite": "^3.2.0-beta.2",
"vite-plugin-dts": "^1.6.6"
"vite-plugin-dts": "^1.7.3"
},
"dependencies": {
"rrweb-snapshot": "^2.0.0-alpha.10"
Expand Down
Loading

0 comments on commit efdc167

Please sign in to comment.