Skip to content

Commit 1060fce

Browse files
dgozmandgozman
authored
feat(selectors): explicit list of custom functions (#4629)
When parsing CSS, we assume everything is a valid CSS function, unless it is in the list of custom functions. This way we'll parse future CSS functions automatically.
1 parent be16ce4 commit 1060fce

File tree

6 files changed

+78
-72
lines changed

6 files changed

+78
-72
lines changed

‎src/debug/injected/consoleApi.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { parseSelector } from '../../server/common/selectorParser';
1817
import type InjectedScript from '../../server/injected/injectedScript';
1918

2019
export class ConsoleAPI {
@@ -32,14 +31,14 @@ export class ConsoleAPI {
3231
_querySelector(selector: string): (Element | undefined) {
3332
if (typeof selector !== 'string')
3433
throw new Error(`Usage: playwright.query('Playwright >> selector').`);
35-
const parsed = parseSelector(selector);
34+
const parsed = this._injectedScript.parseSelector(selector);
3635
return this._injectedScript.querySelector(parsed, document);
3736
}
3837

3938
_querySelectorAll(selector: string): Element[] {
4039
if (typeof selector !== 'string')
4140
throw new Error(`Usage: playwright.$$('Playwright >> selector').`);
42-
const parsed = parseSelector(selector);
41+
const parsed = this._injectedScript.parseSelector(selector);
4342
return this._injectedScript.querySelectorAll(parsed, document);
4443
}
4544

‎src/server/common/cssParser.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ type ClauseCombinator = '' | '>' | '+' | '~';
2121
// - key=value
2222
// - operators like `=`, `|=`, `~=`, `*=`, `/`
2323
// - <empty>~=value
24+
// - argument modes: "parse all", "parse commas", "just a string"
2425
export type CSSFunctionArgument = CSSComplexSelector | number | string;
2526
export type CSSFunction = { name: string, args: CSSFunctionArgument[] };
2627
export type CSSSimpleSelector = { css?: string, functions: CSSFunction[] };
2728
export type CSSComplexSelector = { simples: { selector: CSSSimpleSelector, combinator: ClauseCombinator }[] };
2829
export type CSSComplexSelectorList = CSSComplexSelector[];
2930

30-
export function parseCSS(selector: string): { selector: CSSComplexSelectorList, names: string[] } {
31+
export function parseCSS(selector: string, customNames: Set<string>): { selector: CSSComplexSelectorList, names: string[] } {
3132
let tokens: css.CSSTokenInterface[];
3233
try {
3334
tokens = css.tokenize(selector);
@@ -164,7 +165,7 @@ export function parseCSS(selector: string): { selector: CSSComplexSelectorList,
164165
} else if (tokens[pos] instanceof css.ColonToken) {
165166
pos++;
166167
if (isIdent()) {
167-
if (builtinCSSFilters.has(tokens[pos].value.toLowerCase())) {
168+
if (!customNames.has(tokens[pos].value.toLowerCase())) {
168169
rawCSSString += ':' + tokens[pos++].toSource();
169170
} else {
170171
const name = tokens[pos++].value.toLowerCase();
@@ -173,7 +174,7 @@ export function parseCSS(selector: string): { selector: CSSComplexSelectorList,
173174
}
174175
} else if (tokens[pos] instanceof css.FunctionToken) {
175176
const name = tokens[pos++].value.toLowerCase();
176-
if (builtinCSSFunctions.has(name)) {
177+
if (!customNames.has(name)) {
177178
rawCSSString += `:${name}(${consumeBuiltinFunctionArguments()})`;
178179
} else {
179180
functions.push({ name, args: consumeFunctionArguments() });
@@ -234,16 +235,3 @@ export function serializeSelector(args: CSSFunctionArgument[]) {
234235
}).join(' ');
235236
}).join(', ');
236237
}
237-
238-
const builtinCSSFilters = new Set([
239-
'active', 'any-link', 'checked', 'blank', 'default', 'defined',
240-
'disabled', 'empty', 'enabled', 'first', 'first-child', 'first-of-type',
241-
'fullscreen', 'focus', 'focus-visible', 'focus-within', 'hover',
242-
'indeterminate', 'in-range', 'invalid', 'last-child', 'last-of-type',
243-
'link', 'only-child', 'only-of-type', 'optional', 'out-of-range', 'placeholder-shown',
244-
'read-only', 'read-write', 'required', 'root', 'target', 'valid', 'visited',
245-
]);
246-
247-
const builtinCSSFunctions = new Set([
248-
'dir', 'lang', 'nth-child', 'nth-last-child', 'nth-last-of-type', 'nth-of-type',
249-
]);

‎src/server/common/selectorParser.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,18 @@ export function selectorsV2Enabled() {
3434
return true;
3535
}
3636

37-
export function parseSelector(selector: string): ParsedSelector {
37+
export function selectorsV2EngineNames() {
38+
return ['not', 'is', 'where', 'has', 'scope', 'light', 'index', 'visible', 'matches-text', 'above', 'below', 'right-of', 'left-of', 'near', 'within'];
39+
}
40+
41+
export function parseSelector(selector: string, customNames: Set<string>): ParsedSelector {
3842
const v1 = parseSelectorV1(selector);
39-
const names = new Set<string>(v1.parts.map(part => part.name));
43+
const names = new Set<string>();
44+
for (const { name } of v1.parts) {
45+
names.add(name);
46+
if (!customNames.has(name))
47+
throw new Error(`Unknown engine "${name}" while parsing selector ${selector}`);
48+
}
4049

4150
if (!selectorsV2Enabled()) {
4251
return {
@@ -56,7 +65,7 @@ export function parseSelector(selector: string): ParsedSelector {
5665
}
5766
let simple: CSSSimpleSelector;
5867
if (name === 'css') {
59-
const parsed = parseCSS(part.body);
68+
const parsed = parseCSS(part.body, customNames);
6069
parsed.names.forEach(name => names.add(name));
6170
simple = callWith('is', parsed.selector);
6271
} else if (name === 'text') {

‎src/server/injected/injectedScript.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { createCSSEngine } from './cssSelectorEngine';
1919
import { SelectorEngine, SelectorRoot } from './selectorEngine';
2020
import { createTextSelector } from './textSelectorEngine';
2121
import { XPathEngine } from './xpathSelectorEngine';
22-
import { ParsedSelector, ParsedSelectorV1, parseSelector } from '../common/selectorParser';
22+
import { ParsedSelector, ParsedSelectorV1, parseSelector, selectorsV2Enabled, selectorsV2EngineNames } from '../common/selectorParser';
2323
import { FatalDOMError } from '../common/domErrors';
2424
import { SelectorEvaluatorImpl, SelectorEngine as SelectorEngineV2, QueryContext, isVisible, parentElementOrShadowHost } from './selectorEvaluator';
2525

@@ -43,6 +43,7 @@ export type InjectedScriptPoll<T> = {
4343
export class InjectedScript {
4444
private _enginesV1: Map<string, SelectorEngine>;
4545
private _evaluator: SelectorEvaluatorImpl;
46+
private _engineNames: Set<string>;
4647

4748
constructor(customEngines: { name: string, engine: SelectorEngine}[]) {
4849
this._enginesV1 = new Map();
@@ -67,10 +68,16 @@ export class InjectedScript {
6768
for (const { name, engine } of customEngines)
6869
wrapped.set(name, wrapV2(name, engine));
6970
this._evaluator = new SelectorEvaluatorImpl(wrapped);
71+
72+
this._engineNames = new Set(this._enginesV1.keys());
73+
if (selectorsV2Enabled()) {
74+
for (const name of selectorsV2EngineNames())
75+
this._engineNames.add(name);
76+
}
7077
}
7178

7279
parseSelector(selector: string): ParsedSelector {
73-
return parseSelector(selector);
80+
return parseSelector(selector, this._engineNames);
7481
}
7582

7683
querySelector(selector: ParsedSelector, root: Node): Element | undefined {

‎src/server/selectors.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import * as dom from './dom';
1818
import * as frames from './frames';
1919
import * as js from './javascript';
2020
import * as types from './types';
21-
import { ParsedSelector, parseSelector, selectorsV2Enabled } from './common/selectorParser';
21+
import { ParsedSelector, parseSelector, selectorsV2Enabled, selectorsV2EngineNames } from './common/selectorParser';
2222

2323
export type SelectorInfo = {
2424
parsed: ParsedSelector,
@@ -29,6 +29,7 @@ export type SelectorInfo = {
2929
export class Selectors {
3030
readonly _builtinEngines: Set<string>;
3131
readonly _engines: Map<string, { source: string, contentScript: boolean }>;
32+
readonly _engineNames: Set<string>;
3233

3334
constructor() {
3435
// Note: keep in sync with SelectorEvaluator class.
@@ -42,10 +43,11 @@ export class Selectors {
4243
'data-test', 'data-test:light',
4344
]);
4445
if (selectorsV2Enabled()) {
45-
for (const name of ['not', 'is', 'where', 'has', 'scope', 'light', 'index', 'visible', 'matches-text', 'above', 'below', 'right-of', 'left-of', 'near', 'within'])
46+
for (const name of selectorsV2EngineNames())
4647
this._builtinEngines.add(name);
4748
}
4849
this._engines = new Map();
50+
this._engineNames = new Set(this._builtinEngines);
4951
}
5052

5153
async register(name: string, source: string, contentScript: boolean = false): Promise<void> {
@@ -57,6 +59,7 @@ export class Selectors {
5759
if (this._engines.has(name))
5860
throw new Error(`"${name}" selector engine has been already registered`);
5961
this._engines.set(name, { source, contentScript });
62+
this._engineNames.add(name);
6063
}
6164

6265
async _query(frame: frames.Frame, selector: string, scope?: dom.ElementHandle): Promise<dom.ElementHandle<Element> | null> {
@@ -119,11 +122,7 @@ export class Selectors {
119122
}
120123

121124
_parseSelector(selector: string): SelectorInfo {
122-
const parsed = parseSelector(selector);
123-
for (const name of parsed.names) {
124-
if (!this._builtinEngines.has(name) && !this._engines.has(name))
125-
throw new Error(`Unknown engine "${name}" while parsing selector ${selector}`);
126-
}
125+
const parsed = parseSelector(selector, this._engineNames);
127126
const needsMainWorld = parsed.names.some(name => {
128127
const custom = this._engines.get(name);
129128
return custom ? !custom.contentScript : false;

‎test/css-parser.spec.ts

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,58 +20,62 @@ import * as path from 'path';
2020
const { parseCSS, serializeSelector: serialize } =
2121
require(path.join(__dirname, '..', 'lib', 'server', 'common', 'cssParser'));
2222

23+
const parse = (selector: string) => {
24+
return parseCSS(selector, new Set(['text', 'not', 'has', 'react', 'scope', 'right-of', 'scope', 'is'])).selector;
25+
};
26+
2327
it('should parse css', async () => {
24-
expect(serialize(parseCSS('div').selector)).toBe('div');
25-
expect(serialize(parseCSS('div.class').selector)).toBe('div.class');
26-
expect(serialize(parseCSS('.class').selector)).toBe('.class');
27-
expect(serialize(parseCSS('#id').selector)).toBe('#id');
28-
expect(serialize(parseCSS('.class#id').selector)).toBe('.class#id');
29-
expect(serialize(parseCSS('div#id.class').selector)).toBe('div#id.class');
30-
expect(serialize(parseCSS('*').selector)).toBe('*');
31-
expect(serialize(parseCSS('*div').selector)).toBe('*div');
32-
expect(serialize(parseCSS('div[attr *= foo i]').selector)).toBe('div[attr *= foo i]');
33-
expect(serialize(parseCSS('div[attr~="Bar baz" ]').selector)).toBe('div[attr~="Bar baz" ]');
34-
expect(serialize(parseCSS(`div [ foo = 'bar' s]`).selector)).toBe(`div [ foo = "bar" s]`);
28+
expect(serialize(parse('div'))).toBe('div');
29+
expect(serialize(parse('div.class'))).toBe('div.class');
30+
expect(serialize(parse('.class'))).toBe('.class');
31+
expect(serialize(parse('#id'))).toBe('#id');
32+
expect(serialize(parse('.class#id'))).toBe('.class#id');
33+
expect(serialize(parse('div#id.class'))).toBe('div#id.class');
34+
expect(serialize(parse('*'))).toBe('*');
35+
expect(serialize(parse('*div'))).toBe('*div');
36+
expect(serialize(parse('div[attr *= foo i]'))).toBe('div[attr *= foo i]');
37+
expect(serialize(parse('div[attr~="Bar baz" ]'))).toBe('div[attr~="Bar baz" ]');
38+
expect(serialize(parse(`div [ foo = 'bar' s]`))).toBe(`div [ foo = "bar" s]`);
3539

36-
expect(serialize(parseCSS(':hover').selector)).toBe(':hover');
37-
expect(serialize(parseCSS('div:hover').selector)).toBe('div:hover');
38-
expect(serialize(parseCSS('#id:active:hover').selector)).toBe('#id:active:hover');
39-
expect(serialize(parseCSS(':dir(ltr)').selector)).toBe(':dir(ltr)');
40-
expect(serialize(parseCSS('#foo-bar.cls:nth-child(3n + 10)').selector)).toBe('#foo-bar.cls:nth-child(3n + 10)');
41-
expect(serialize(parseCSS(':lang(en)').selector)).toBe(':lang(en)');
42-
expect(serialize(parseCSS('*:hover').selector)).toBe('*:hover');
40+
expect(serialize(parse(':hover'))).toBe(':hover');
41+
expect(serialize(parse('div:hover'))).toBe('div:hover');
42+
expect(serialize(parse('#id:active:hover'))).toBe('#id:active:hover');
43+
expect(serialize(parse(':dir(ltr)'))).toBe(':dir(ltr)');
44+
expect(serialize(parse('#foo-bar.cls:nth-child(3n + 10)'))).toBe('#foo-bar.cls:nth-child(3n + 10)');
45+
expect(serialize(parse(':lang(en)'))).toBe(':lang(en)');
46+
expect(serialize(parse('*:hover'))).toBe('*:hover');
4347

44-
expect(serialize(parseCSS('div span').selector)).toBe('div span');
45-
expect(serialize(parseCSS('div>span').selector)).toBe('div > span');
46-
expect(serialize(parseCSS('div +span').selector)).toBe('div + span');
47-
expect(serialize(parseCSS('div~ span').selector)).toBe('div ~ span');
48-
expect(serialize(parseCSS('div >.class #id+ span').selector)).toBe('div > .class #id + span');
49-
expect(serialize(parseCSS('div>span+.class').selector)).toBe('div > span + .class');
48+
expect(serialize(parse('div span'))).toBe('div span');
49+
expect(serialize(parse('div>span'))).toBe('div > span');
50+
expect(serialize(parse('div +span'))).toBe('div + span');
51+
expect(serialize(parse('div~ span'))).toBe('div ~ span');
52+
expect(serialize(parse('div >.class #id+ span'))).toBe('div > .class #id + span');
53+
expect(serialize(parse('div>span+.class'))).toBe('div > span + .class');
5054

51-
expect(serialize(parseCSS('div:not(span)').selector)).toBe('div:not(span)');
52-
expect(serialize(parseCSS(':not(span)#id').selector)).toBe('#id:not(span)');
53-
expect(serialize(parseCSS('div:not(span):hover').selector)).toBe('div:hover:not(span)');
54-
expect(serialize(parseCSS('div:has(span):hover').selector)).toBe('div:hover:has(span)');
55-
expect(serialize(parseCSS('div:right-of(span):hover').selector)).toBe('div:hover:right-of(span)');
56-
expect(serialize(parseCSS(':right-of(span):react(foobar)').selector)).toBe(':right-of(span):react(foobar)');
57-
expect(serialize(parseCSS('div:is(span):hover').selector)).toBe('div:hover:is(span)');
58-
expect(serialize(parseCSS('div:scope:hover').selector)).toBe('div:hover:scope()');
59-
expect(serialize(parseCSS('div:sCOpe:HOVER').selector)).toBe('div:HOVER:scope()');
60-
expect(serialize(parseCSS('div:NOT(span):hoVER').selector)).toBe('div:hoVER:not(span)');
55+
expect(serialize(parse('div:not(span)'))).toBe('div:not(span)');
56+
expect(serialize(parse(':not(span)#id'))).toBe('#id:not(span)');
57+
expect(serialize(parse('div:not(span):hover'))).toBe('div:hover:not(span)');
58+
expect(serialize(parse('div:has(span):hover'))).toBe('div:hover:has(span)');
59+
expect(serialize(parse('div:right-of(span):hover'))).toBe('div:hover:right-of(span)');
60+
expect(serialize(parse(':right-of(span):react(foobar)'))).toBe(':right-of(span):react(foobar)');
61+
expect(serialize(parse('div:is(span):hover'))).toBe('div:hover:is(span)');
62+
expect(serialize(parse('div:scope:hover'))).toBe('div:hover:scope()');
63+
expect(serialize(parse('div:sCOpe:HOVER'))).toBe('div:HOVER:scope()');
64+
expect(serialize(parse('div:NOT(span):hoVER'))).toBe('div:hoVER:not(span)');
6165

62-
expect(serialize(parseCSS(':text("foo")').selector)).toBe(':text("foo")');
63-
expect(serialize(parseCSS(':text("*")').selector)).toBe(':text("*")');
64-
expect(serialize(parseCSS(':text(*)').selector)).toBe(':text(*)');
65-
expect(serialize(parseCSS(':text("foo", normalize-space)').selector)).toBe(':text("foo", normalize-space)');
66-
expect(serialize(parseCSS(':index(3, div span)').selector)).toBe(':index(3, div span)');
67-
expect(serialize(parseCSS(':is(foo, bar>baz.cls+:not(qux))').selector)).toBe(':is(foo, bar > baz.cls + :not(qux))');
66+
expect(serialize(parse(':text("foo")'))).toBe(':text("foo")');
67+
expect(serialize(parse(':text("*")'))).toBe(':text("*")');
68+
expect(serialize(parse(':text(*)'))).toBe(':text(*)');
69+
expect(serialize(parse(':text("foo", normalize-space)'))).toBe(':text("foo", normalize-space)');
70+
expect(serialize(parse(':index(3, div span)'))).toBe(':index(3, div span)');
71+
expect(serialize(parse(':is(foo, bar>baz.cls+:not(qux))'))).toBe(':is(foo, bar > baz.cls + :not(qux))');
6872
});
6973

7074
it('should throw on malformed css', async () => {
7175
function expectError(selector: string) {
7276
let error = { message: '' };
7377
try {
74-
parseCSS(selector);
78+
parse(selector);
7579
} catch (e) {
7680
error = e;
7781
}

0 commit comments

Comments
 (0)