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 <eoghanmurray@users.noreply.github.com>
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
This commit is contained in:
7
.changeset/rotten-spies-enjoy.md
Normal file
7
.changeset/rotten-spies-enjoy.md
Normal file
@@ -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
|
||||
@@ -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[];
|
||||
@@ -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>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -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.
|
||||
@@ -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>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -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. */
|
||||
@@ -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[];
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in New Issue
Block a user