From 7700d91c078abc083c27c81fb7908f2368921a07 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 4 Jun 2024 20:05:43 -0400 Subject: [PATCH] Revert "Fix some css issues with :hover and rewrite max-device-width (#1431)" This reverts commit a7c33f2093c4d92faf7ae25e8bb0e088d122c13b. --- .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, 57 insertions(+), 133 deletions(-) delete mode 100644 .changeset/rotten-spies-enjoy.md diff --git a/.changeset/rotten-spies-enjoy.md b/.changeset/rotten-spies-enjoy.md deleted file mode 100644 index 5f5954fcac..0000000000 --- a/.changeset/rotten-spies-enjoy.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'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 1a3157d40f..ee36a4771e 100644 --- a/packages/rrweb-snapshot/src/css.ts +++ b/packages/rrweb-snapshot/src/css.ts @@ -56,11 +56,6 @@ 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[]; @@ -103,11 +98,13 @@ export interface CustomMedia extends Node { /** * The @document at-rule. */ -export interface Document extends NodeWithRules { +export interface Document extends Node { /** 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; } /** @@ -121,7 +118,10 @@ export interface FontFace extends Node { /** * The @host at-rule. */ -export type Host = NodeWithRules; +export interface Host extends Node { + /** Array of nodes with the types rule, comment and any of the at-rule types. */ + rules?: Array; +} /** * The @import at-rule. @@ -153,9 +153,11 @@ export interface KeyFrame extends Node { /** * The @media at-rule. */ -export interface Media extends NodeWithRules { +export interface Media extends Node { /** The part following @media. */ media?: string; + /** Array of nodes with the types rule, comment and any of the at-rule types. */ + rules?: Array; } /** @@ -179,9 +181,11 @@ export interface Page extends Node { /** * The @supports at-rule. */ -export interface Supports extends NodeWithRules { +export interface Supports extends Node { /** 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. */ @@ -201,8 +205,10 @@ export type AtRule = /** * A collection of rules */ -export interface StyleRules extends NodeWithRules { +export interface StyleRules { 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[]; } @@ -218,7 +224,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 = {}): Stylesheet { +export function parse(css: string, options: ParserOptions = {}) { /** * Positional. */ @@ -936,7 +942,7 @@ function trim(str: string) { * Adds non-enumerable parent node reference to each node. */ -function addParent(obj: Stylesheet, parent?: Stylesheet): Stylesheet { +function addParent(obj: Stylesheet, parent?: 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 8b3e6d2eef..798e4458d2 100644 --- a/packages/rrweb-snapshot/src/index.ts +++ b/packages/rrweb-snapshot/src/index.ts @@ -13,7 +13,7 @@ import snapshot, { } from './snapshot'; import rebuild, { buildNodeWithSN, - adaptCssForReplay, + addHoverClass, createCache, } from './rebuild'; export * from './types'; @@ -24,7 +24,7 @@ export { serializeNodeWithId, rebuild, buildNodeWithSN, - adaptCssForReplay, + addHoverClass, createCache, transformAttribute, ignoreAttribute, diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts index cc16bf3497..bda5715adc 100644 --- a/packages/rrweb-snapshot/src/rebuild.ts +++ b/packages/rrweb-snapshot/src/rebuild.ts @@ -1,4 +1,4 @@ -import { Rule, Media, NodeWithRules, parse } from './css'; +import { parse } from './css'; import { serializedNodeWithId, NodeType, @@ -63,11 +63,9 @@ 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 adaptCssForReplay(cssText: string, cache: BuildCache): string { +export function addHoverClass(cssText: string, cache: BuildCache): string { const cachedStyle = cache?.stylesWithHoverClass.get(cssText); if (cachedStyle) return cachedStyle; @@ -86,61 +84,35 @@ export function adaptCssForReplay(cssText: string, cache: BuildCache): string { } const selectors: string[] = []; - const medias: string[] = []; - function getSelectors(rule: Rule | Media | NodeWithRules) { - if ('selectors' in rule && rule.selectors) { - rule.selectors.forEach((selector: string) => { + ast.stylesheet.rules.forEach((rule) => { + if ('selectors' in rule) { + (rule.selectors || []).forEach((selector: string) => { if (HOVER_SELECTOR.test(selector)) { selectors.push(selector); } }); } - 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); + }); - 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'); - }); + if (selectors.length === 0) { + return cssText; } + + 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}`; + }); cache?.stylesWithHoverClass.set(cssText, result); return result; } @@ -231,7 +203,7 @@ function buildNode( const isTextarea = tagName === 'textarea' && name === 'value'; const isRemoteOrDynamicCss = tagName === 'style' && name === '_cssText'; if (isRemoteOrDynamicCss && hackCss && typeof value === 'string') { - value = adaptCssForReplay(value, cache); + value = addHoverClass(value, cache); } if ((isTextarea || isRemoteOrDynamicCss) && typeof value === 'string') { const child = doc.createTextNode(value); @@ -381,7 +353,7 @@ function buildNode( case NodeType.Text: return doc.createTextNode( n.isStyle && hackCss - ? adaptCssForReplay(n.textContent, cache) + ? addHoverClass(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 e923e7cb45..50faad38e8 100644 --- a/packages/rrweb-snapshot/test/rebuild.test.ts +++ b/packages/rrweb-snapshot/test/rebuild.test.ts @@ -3,11 +3,7 @@ */ import * as fs from 'fs'; import * as path from 'path'; -import { - adaptCssForReplay, - buildNodeWithSN, - createCache, -} from '../src/rebuild'; +import { addHoverClass, buildNodeWithSN, createCache } from '../src/rebuild'; import { NodeType } from '../src/types'; import { createMirror, Mirror } from '../src/utils'; @@ -111,90 +107,47 @@ 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(adaptCssForReplay(cssText, cache)).toEqual(cssText); + expect(addHoverClass(cssText, cache)).toEqual(cssText); }); it('can add hover class to css text', () => { const cssText = '.a:hover { color: white }'; - expect(adaptCssForReplay(cssText, cache)).toEqual( + expect(addHoverClass(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(adaptCssForReplay(cssText, cache)).toEqual( + expect(addHoverClass(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(adaptCssForReplay(cssText, cache)).toEqual( + expect(addHoverClass(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(adaptCssForReplay(cssText, cache)).toEqual( + expect(addHoverClass(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(adaptCssForReplay(cssText, cache)).toEqual( + expect(addHoverClass(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(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; }}', - ); + expect(addHoverClass(cssText, cache)).toEqual(cssText); }); // this benchmark is unreliable when run in parallel with other tests @@ -204,7 +157,7 @@ ul li.specified c:hover img, ul li.specified c.\\:hover img { 'utf8', ); const start = process.hrtime(); - adaptCssForReplay(cssText, cache); + addHoverClass(cssText, cache); const end = process.hrtime(start); const duration = getDuration(end); expect(duration).toBeLessThan(100); @@ -219,11 +172,11 @@ ul li.specified c:hover img, ul li.specified c.\\:hover img { ); const start = process.hrtime(); - adaptCssForReplay(cssText, cache); + addHoverClass(cssText, cache); const end = process.hrtime(start); const cachedStart = process.hrtime(); - adaptCssForReplay(cssText, cache); + addHoverClass(cssText, cache); const cachedEnd = process.hrtime(cachedStart); expect(getDuration(cachedEnd) * factor).toBeLessThan(getDuration(end));