From a7c33f2093c4d92faf7ae25e8bb0e088d122c13b Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Tue, 26 Mar 2024 15:44:43 +0000 Subject: [PATCH] Fix some css issues with :hover and rewrite max-device-width (#1431) * We weren't recursing into media queries (or @supports etc.) to rewrite hover pseudoclasses * The early return meant that the stylesWithHoverClass cache wasn't being populated if there were no hover selectors on the stylesheet - not committing the test, but modifying the existing 'add a hover class to a previously processed css string' as follows shows the problem: --- a/packages/rrweb-snapshot/test/rebuild.test.ts +++ b/packages/rrweb-snapshot/test/rebuild.test.ts @@ -151,6 +185,7 @@ describe('rebuild', function () { path.resolve(__dirname, './css/benchmark.css'), 'utf8', ); + cssText = cssText.replace(/:hover/g, ''); const start = process.hrtime(); addHoverClass(cssText, cache); * Replace `min-device-width` and similar with `min-width` as the former looks out at the browser viewport whereas we need it to look at the replayer iframe viewport * Add some tests to show how the hover replacement works against selector lists. I believe these were failing in a previous version of rrweb as I had some local patches that no longer seem to be needed to handle these cases * Update name of function to reflect that 'addHoverClass' does more than just :hover. I believe this function is only exported for the purposes of use in the tests * Apply formatting changes * Create rotten-spies-enjoy.md * Apply formatting changes * Add correct typing on `getSelectors` * Refactor CSS interfaces to include optional rules * Change `rules` to be non optional --------- Co-authored-by: eoghanmurray Co-authored-by: Justin Halsall --- .changeset/rotten-spies-enjoy.md | 7 ++ packages/rrweb-snapshot/src/css.ts | 30 +++----- packages/rrweb-snapshot/src/index.ts | 4 +- packages/rrweb-snapshot/src/rebuild.ts | 80 +++++++++++++------- packages/rrweb-snapshot/test/rebuild.test.ts | 69 ++++++++++++++--- 5 files changed, 133 insertions(+), 57 deletions(-) create mode 100644 .changeset/rotten-spies-enjoy.md diff --git a/.changeset/rotten-spies-enjoy.md b/.changeset/rotten-spies-enjoy.md new file mode 100644 index 0000000000..5f5954fcac --- /dev/null +++ b/.changeset/rotten-spies-enjoy.md @@ -0,0 +1,7 @@ +--- +'rrweb-snapshot': patch +'rrweb': patch +--- + +Ensure :hover works on replayer, even if a rule is behind a media query +Respect the intent behind max-device-width and min-device-width media queries so that their effects are apparent in the replayer context diff --git a/packages/rrweb-snapshot/src/css.ts b/packages/rrweb-snapshot/src/css.ts index d7a413eb67..522e257daf 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -56,6 +56,11 @@ export interface Node { }; } +export interface NodeWithRules extends Node { + /** Array of nodes with the types rule, comment and any of the at-rule types. */ + rules: Array; +} + export interface Rule extends Node { /** The list of selectors of the rule, split on commas. Each selector is trimmed from whitespace and comments. */ selectors?: string[]; @@ -98,13 +103,11 @@ export interface CustomMedia extends Node { /** * The @document at-rule. */ -export interface Document extends Node { +export interface Document extends NodeWithRules { /** The part following @document. */ document?: string; /** The vendor prefix in @document, or undefined if there is none. */ vendor?: string; - /** Array of nodes with the types rule, comment and any of the at-rule types. */ - rules?: Array; } /** @@ -118,10 +121,7 @@ export interface FontFace extends Node { /** * The @host at-rule. */ -export interface Host extends Node { - /** Array of nodes with the types rule, comment and any of the at-rule types. */ - rules?: Array; -} +export type Host = NodeWithRules; /** * The @import at-rule. @@ -153,11 +153,9 @@ export interface KeyFrame extends Node { /** * The @media at-rule. */ -export interface Media extends Node { +export interface Media extends NodeWithRules { /** The part following @media. */ media?: string; - /** Array of nodes with the types rule, comment and any of the at-rule types. */ - rules?: Array; } /** @@ -181,11 +179,9 @@ export interface Page extends Node { /** * The @supports at-rule. */ -export interface Supports extends Node { +export interface Supports extends NodeWithRules { /** The part following @supports. */ supports?: string; - /** Array of nodes with the types rule, comment and any of the at-rule types. */ - rules?: Array; } /** All at-rules. */ @@ -205,10 +201,8 @@ export type AtRule = /** * A collection of rules */ -export interface StyleRules { +export interface StyleRules extends NodeWithRules { source?: string; - /** Array of nodes with the types rule, comment and any of the at-rule types. */ - rules: Array; /** Array of Errors. Errors collected during parsing when option silent is true. */ parsingErrors?: ParserError[]; } @@ -224,7 +218,7 @@ export interface Stylesheet extends Node { // https://github.com/visionmedia/css-parse/pull/49#issuecomment-30088027 const commentre = /\/\*[^*]*\*+([^/*][^*]*\*+)*\//g; -export function parse(css: string, options: ParserOptions = {}) { +export function parse(css: string, options: ParserOptions = {}): Stylesheet { /** * Positional. */ @@ -882,7 +876,7 @@ function trim(str: string) { * Adds non-enumerable parent node reference to each node. */ -function addParent(obj: Stylesheet, parent?: Stylesheet) { +function addParent(obj: Stylesheet, parent?: Stylesheet): Stylesheet { const isNode = obj && typeof obj.type === 'string'; const childParent = isNode ? obj : parent; diff --git a/packages/rrweb-snapshot/src/index.ts b/packages/rrweb-snapshot/src/index.ts index ef9d1b19fa..c9f91a9100 100644 --- a/packages/rrweb-snapshot/src/index.ts +++ b/packages/rrweb-snapshot/src/index.ts @@ -11,7 +11,7 @@ import snapshot, { } from './snapshot'; import rebuild, { buildNodeWithSN, - addHoverClass, + adaptCssForReplay, createCache, } from './rebuild'; export * from './types'; @@ -22,7 +22,7 @@ export { serializeNodeWithId, rebuild, buildNodeWithSN, - addHoverClass, + adaptCssForReplay, createCache, transformAttribute, ignoreAttribute, diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts index 10bf067a76..dc9b6c3b9f 100644 --- a/packages/rrweb-snapshot/src/rebuild.ts +++ b/packages/rrweb-snapshot/src/rebuild.ts @@ -1,4 +1,4 @@ -import { parse } from './css'; +import { Rule, Media, NodeWithRules, parse } from './css'; import { serializedNodeWithId, NodeType, @@ -62,9 +62,11 @@ function escapeRegExp(str: string) { return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string } +const MEDIA_SELECTOR = /(max|min)-device-(width|height)/; +const MEDIA_SELECTOR_GLOBAL = new RegExp(MEDIA_SELECTOR.source, 'g'); const HOVER_SELECTOR = /([^\\]):hover/; const HOVER_SELECTOR_GLOBAL = new RegExp(HOVER_SELECTOR.source, 'g'); -export function addHoverClass(cssText: string, cache: BuildCache): string { +export function adaptCssForReplay(cssText: string, cache: BuildCache): string { const cachedStyle = cache?.stylesWithHoverClass.get(cssText); if (cachedStyle) return cachedStyle; @@ -77,35 +79,61 @@ export function addHoverClass(cssText: string, cache: BuildCache): string { } const selectors: string[] = []; - ast.stylesheet.rules.forEach((rule) => { - if ('selectors' in rule) { - (rule.selectors || []).forEach((selector: string) => { + const medias: string[] = []; + function getSelectors(rule: Rule | Media | NodeWithRules) { + if ('selectors' in rule && rule.selectors) { + rule.selectors.forEach((selector: string) => { if (HOVER_SELECTOR.test(selector)) { selectors.push(selector); } }); } - }); - - if (selectors.length === 0) { - return cssText; + if ('media' in rule && rule.media && MEDIA_SELECTOR.test(rule.media)) { + medias.push(rule.media); + } + if ('rules' in rule && rule.rules) { + rule.rules.forEach(getSelectors); + } } + getSelectors(ast.stylesheet); - const selectorMatcher = new RegExp( - selectors - .filter((selector, index) => selectors.indexOf(selector) === index) - .sort((a, b) => b.length - a.length) - .map((selector) => { - return escapeRegExp(selector); - }) - .join('|'), - 'g', - ); - - const result = cssText.replace(selectorMatcher, (selector) => { - const newSelector = selector.replace(HOVER_SELECTOR_GLOBAL, '$1.\\:hover'); - return `${selector}, ${newSelector}`; - }); + let result = cssText; + if (selectors.length > 0) { + const selectorMatcher = new RegExp( + selectors + .filter((selector, index) => selectors.indexOf(selector) === index) + .sort((a, b) => b.length - a.length) + .map((selector) => { + return escapeRegExp(selector); + }) + .join('|'), + 'g', + ); + result = result.replace(selectorMatcher, (selector) => { + const newSelector = selector.replace( + HOVER_SELECTOR_GLOBAL, + '$1.\\:hover', + ); + return `${selector}, ${newSelector}`; + }); + } + if (medias.length > 0) { + const mediaMatcher = new RegExp( + medias + .filter((media, index) => medias.indexOf(media) === index) + .sort((a, b) => b.length - a.length) + .map((media) => { + return escapeRegExp(media); + }) + .join('|'), + 'g', + ); + result = result.replace(mediaMatcher, (media) => { + // not attempting to maintain min-device-width along with min-width + // (it's non standard) + return media.replace(MEDIA_SELECTOR_GLOBAL, '$1-$2'); + }); + } cache?.stylesWithHoverClass.set(cssText, result); return result; } @@ -196,7 +224,7 @@ function buildNode( const isTextarea = tagName === 'textarea' && name === 'value'; const isRemoteOrDynamicCss = tagName === 'style' && name === '_cssText'; if (isRemoteOrDynamicCss && hackCss && typeof value === 'string') { - value = addHoverClass(value, cache); + value = adaptCssForReplay(value, cache); } if ((isTextarea || isRemoteOrDynamicCss) && typeof value === 'string') { node.appendChild(doc.createTextNode(value)); @@ -341,7 +369,7 @@ function buildNode( case NodeType.Text: return doc.createTextNode( n.isStyle && hackCss - ? addHoverClass(n.textContent, cache) + ? adaptCssForReplay(n.textContent, cache) : n.textContent, ); case NodeType.CDATA: diff --git a/packages/rrweb-snapshot/test/rebuild.test.ts b/packages/rrweb-snapshot/test/rebuild.test.ts index 357cd2fb3c..097ff0989a 100644 --- a/packages/rrweb-snapshot/test/rebuild.test.ts +++ b/packages/rrweb-snapshot/test/rebuild.test.ts @@ -3,7 +3,11 @@ */ import * as fs from 'fs'; import * as path from 'path'; -import { addHoverClass, buildNodeWithSN, createCache } from '../src/rebuild'; +import { + adaptCssForReplay, + buildNodeWithSN, + createCache, +} from '../src/rebuild'; import { NodeType } from '../src/types'; import { createMirror, Mirror } from '../src/utils'; @@ -81,47 +85,90 @@ describe('rebuild', function () { describe('add hover class to hover selector related rules', function () { it('will do nothing to css text without :hover', () => { const cssText = 'body { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual(cssText); + expect(adaptCssForReplay(cssText, cache)).toEqual(cssText); }); it('can add hover class to css text', () => { const cssText = '.a:hover { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual( + expect(adaptCssForReplay(cssText, cache)).toEqual( '.a:hover, .a.\\:hover { color: white }', ); }); + it('can correctly add hover when in middle of selector', () => { + const cssText = 'ul li a:hover img { color: white }'; + expect(adaptCssForReplay(cssText, cache)).toEqual( + 'ul li a:hover img, ul li a.\\:hover img { color: white }', + ); + }); + + it('can correctly add hover on multiline selector', () => { + const cssText = `ul li.specified a:hover img, +ul li.multiline +b:hover +img, +ul li.specified c:hover img { + color: white +}`; + expect(adaptCssForReplay(cssText, cache)).toEqual( + `ul li.specified a:hover img, ul li.specified a.\\:hover img, +ul li.multiline +b:hover +img, ul li.multiline +b.\\:hover +img, +ul li.specified c:hover img, ul li.specified c.\\:hover img { + color: white +}`, + ); + }); + + it('can add hover class within media query', () => { + const cssText = '@media screen { .m:hover { color: white } }'; + expect(adaptCssForReplay(cssText, cache)).toEqual( + '@media screen { .m:hover, .m.\\:hover { color: white } }', + ); + }); + it('can add hover class when there is multi selector', () => { const cssText = '.a, .b:hover, .c { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual( + expect(adaptCssForReplay(cssText, cache)).toEqual( '.a, .b:hover, .b.\\:hover, .c { color: white }', ); }); it('can add hover class when there is a multi selector with the same prefix', () => { const cssText = '.a:hover, .a:hover::after { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual( + expect(adaptCssForReplay(cssText, cache)).toEqual( '.a:hover, .a.\\:hover, .a:hover::after, .a.\\:hover::after { color: white }', ); }); it('can add hover class when :hover is not the end of selector', () => { const cssText = 'div:hover::after { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual( + expect(adaptCssForReplay(cssText, cache)).toEqual( 'div:hover::after, div.\\:hover::after { color: white }', ); }); it('can add hover class when the selector has multi :hover', () => { const cssText = 'a:hover b:hover { color: white }'; - expect(addHoverClass(cssText, cache)).toEqual( + expect(adaptCssForReplay(cssText, cache)).toEqual( 'a:hover b:hover, a.\\:hover b.\\:hover { color: white }', ); }); it('will ignore :hover in css value', () => { const cssText = '.a::after { content: ":hover" }'; - expect(addHoverClass(cssText, cache)).toEqual(cssText); + expect(adaptCssForReplay(cssText, cache)).toEqual(cssText); + }); + + it('can adapt media rules to replay context', () => { + const cssText = + '@media only screen and (min-device-width : 1200px) { .a { width: 10px; }}'; + expect(adaptCssForReplay(cssText, cache)).toEqual( + '@media only screen and (min-width : 1200px) { .a { width: 10px; }}', + ); }); // this benchmark is unreliable when run in parallel with other tests @@ -131,7 +178,7 @@ describe('rebuild', function () { 'utf8', ); const start = process.hrtime(); - addHoverClass(cssText, cache); + adaptCssForReplay(cssText, cache); const end = process.hrtime(start); const duration = getDuration(end); expect(duration).toBeLessThan(100); @@ -146,11 +193,11 @@ describe('rebuild', function () { ); const start = process.hrtime(); - addHoverClass(cssText, cache); + adaptCssForReplay(cssText, cache); const end = process.hrtime(start); const cachedStart = process.hrtime(); - addHoverClass(cssText, cache); + adaptCssForReplay(cssText, cache); const cachedEnd = process.hrtime(cachedStart); expect(getDuration(cachedEnd) * factor).toBeLessThan(getDuration(end));