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.

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 some css issues with :hover and rewrite max-device-width #1431

Merged
merged 11 commits into from
Mar 26, 2024
Merged
7 changes: 7 additions & 0 deletions .changeset/rotten-spies-enjoy.md
Original file line number Diff line number Diff line change
@@ -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
30 changes: 12 additions & 18 deletions packages/rrweb-snapshot/src/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Rule | Comment | AtRule>;
}

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[];
Expand Down Expand Up @@ -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<Rule | Comment | AtRule>;
}

/**
Expand All @@ -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<Rule | Comment | AtRule>;
}
export type Host = NodeWithRules;

/**
* The @import at-rule.
Expand Down Expand Up @@ -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<Rule | Comment | AtRule>;
}

/**
Expand All @@ -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<Rule | Comment | AtRule>;
}

/** All at-rules. */
Expand All @@ -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<Rule | Comment | AtRule>;
/** Array of Errors. Errors collected during parsing when option silent is true. */
parsingErrors?: ParserError[];
}
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb-snapshot/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import snapshot, {
} from './snapshot';
import rebuild, {
buildNodeWithSN,
addHoverClass,
adaptCssForReplay,
createCache,
} from './rebuild';
export * from './types';
Expand All @@ -22,7 +22,7 @@ export {
serializeNodeWithId,
rebuild,
buildNodeWithSN,
addHoverClass,
adaptCssForReplay,
createCache,
transformAttribute,
ignoreAttribute,
Expand Down
80 changes: 54 additions & 26 deletions packages/rrweb-snapshot/src/rebuild.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { parse } from './css';
import { Rule, Media, NodeWithRules, parse } from './css';
import {
serializedNodeWithId,
NodeType,
Expand Down Expand Up @@ -62,9 +62,11 @@
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;

Expand All @@ -77,35 +79,61 @@
}

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;
}
Expand Down Expand Up @@ -196,7 +224,7 @@
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));
Expand Down Expand Up @@ -341,7 +369,7 @@
case NodeType.Text:
return doc.createTextNode(
n.isStyle && hackCss
? addHoverClass(n.textContent, cache)
? adaptCssForReplay(n.textContent, cache)
: n.textContent,
);
case NodeType.CDATA:
Expand Down Expand Up @@ -490,7 +518,7 @@

for (const id of mirror.getIds()) {
if (mirror.has(id)) {
walk(mirror.getNode(id)!);

Check warning on line 521 in packages/rrweb-snapshot/src/rebuild.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/rebuild.ts#L521

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
}
}
}
Expand Down
69 changes: 58 additions & 11 deletions packages/rrweb-snapshot/test/rebuild.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
}`,
Comment on lines +107 to +122
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can add whitespace here to make the tests a little less messy, unless I'm missing something it should be fine.

Suggested change
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
}`,
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
Expand All @@ -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);
Expand All @@ -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));
Expand Down
Loading