From 1060fce005a9b0f33bdff1080f64051c6aab026b Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 8 Dec 2020 09:38:43 -0800 Subject: [PATCH] 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. --- src/debug/injected/consoleApi.ts | 5 +- src/server/common/cssParser.ts | 20 ++----- src/server/common/selectorParser.ts | 15 ++++- src/server/injected/injectedScript.ts | 11 +++- src/server/selectors.ts | 13 ++-- test/css-parser.spec.ts | 86 ++++++++++++++------------- 6 files changed, 78 insertions(+), 72 deletions(-) diff --git a/src/debug/injected/consoleApi.ts b/src/debug/injected/consoleApi.ts index abad64a101..6ef01d8c0d 100644 --- a/src/debug/injected/consoleApi.ts +++ b/src/debug/injected/consoleApi.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { parseSelector } from '../../server/common/selectorParser'; import type InjectedScript from '../../server/injected/injectedScript'; export class ConsoleAPI { @@ -32,14 +31,14 @@ export class ConsoleAPI { _querySelector(selector: string): (Element | undefined) { if (typeof selector !== 'string') throw new Error(`Usage: playwright.query('Playwright >> selector').`); - const parsed = parseSelector(selector); + const parsed = this._injectedScript.parseSelector(selector); return this._injectedScript.querySelector(parsed, document); } _querySelectorAll(selector: string): Element[] { if (typeof selector !== 'string') throw new Error(`Usage: playwright.$$('Playwright >> selector').`); - const parsed = parseSelector(selector); + const parsed = this._injectedScript.parseSelector(selector); return this._injectedScript.querySelectorAll(parsed, document); } diff --git a/src/server/common/cssParser.ts b/src/server/common/cssParser.ts index 3bcd14738c..f98da9fb67 100644 --- a/src/server/common/cssParser.ts +++ b/src/server/common/cssParser.ts @@ -21,13 +21,14 @@ type ClauseCombinator = '' | '>' | '+' | '~'; // - key=value // - operators like `=`, `|=`, `~=`, `*=`, `/` // - ~=value +// - argument modes: "parse all", "parse commas", "just a string" export type CSSFunctionArgument = CSSComplexSelector | number | string; export type CSSFunction = { name: string, args: CSSFunctionArgument[] }; export type CSSSimpleSelector = { css?: string, functions: CSSFunction[] }; export type CSSComplexSelector = { simples: { selector: CSSSimpleSelector, combinator: ClauseCombinator }[] }; export type CSSComplexSelectorList = CSSComplexSelector[]; -export function parseCSS(selector: string): { selector: CSSComplexSelectorList, names: string[] } { +export function parseCSS(selector: string, customNames: Set): { selector: CSSComplexSelectorList, names: string[] } { let tokens: css.CSSTokenInterface[]; try { tokens = css.tokenize(selector); @@ -164,7 +165,7 @@ export function parseCSS(selector: string): { selector: CSSComplexSelectorList, } else if (tokens[pos] instanceof css.ColonToken) { pos++; if (isIdent()) { - if (builtinCSSFilters.has(tokens[pos].value.toLowerCase())) { + if (!customNames.has(tokens[pos].value.toLowerCase())) { rawCSSString += ':' + tokens[pos++].toSource(); } else { const name = tokens[pos++].value.toLowerCase(); @@ -173,7 +174,7 @@ export function parseCSS(selector: string): { selector: CSSComplexSelectorList, } } else if (tokens[pos] instanceof css.FunctionToken) { const name = tokens[pos++].value.toLowerCase(); - if (builtinCSSFunctions.has(name)) { + if (!customNames.has(name)) { rawCSSString += `:${name}(${consumeBuiltinFunctionArguments()})`; } else { functions.push({ name, args: consumeFunctionArguments() }); @@ -234,16 +235,3 @@ export function serializeSelector(args: CSSFunctionArgument[]) { }).join(' '); }).join(', '); } - -const builtinCSSFilters = new Set([ - 'active', 'any-link', 'checked', 'blank', 'default', 'defined', - 'disabled', 'empty', 'enabled', 'first', 'first-child', 'first-of-type', - 'fullscreen', 'focus', 'focus-visible', 'focus-within', 'hover', - 'indeterminate', 'in-range', 'invalid', 'last-child', 'last-of-type', - 'link', 'only-child', 'only-of-type', 'optional', 'out-of-range', 'placeholder-shown', - 'read-only', 'read-write', 'required', 'root', 'target', 'valid', 'visited', -]); - -const builtinCSSFunctions = new Set([ - 'dir', 'lang', 'nth-child', 'nth-last-child', 'nth-last-of-type', 'nth-of-type', -]); diff --git a/src/server/common/selectorParser.ts b/src/server/common/selectorParser.ts index a12dd6c304..029618ab46 100644 --- a/src/server/common/selectorParser.ts +++ b/src/server/common/selectorParser.ts @@ -34,9 +34,18 @@ export function selectorsV2Enabled() { return true; } -export function parseSelector(selector: string): ParsedSelector { +export function selectorsV2EngineNames() { + return ['not', 'is', 'where', 'has', 'scope', 'light', 'index', 'visible', 'matches-text', 'above', 'below', 'right-of', 'left-of', 'near', 'within']; +} + +export function parseSelector(selector: string, customNames: Set): ParsedSelector { const v1 = parseSelectorV1(selector); - const names = new Set(v1.parts.map(part => part.name)); + const names = new Set(); + for (const { name } of v1.parts) { + names.add(name); + if (!customNames.has(name)) + throw new Error(`Unknown engine "${name}" while parsing selector ${selector}`); + } if (!selectorsV2Enabled()) { return { @@ -56,7 +65,7 @@ export function parseSelector(selector: string): ParsedSelector { } let simple: CSSSimpleSelector; if (name === 'css') { - const parsed = parseCSS(part.body); + const parsed = parseCSS(part.body, customNames); parsed.names.forEach(name => names.add(name)); simple = callWith('is', parsed.selector); } else if (name === 'text') { diff --git a/src/server/injected/injectedScript.ts b/src/server/injected/injectedScript.ts index d111cac270..5d099bad92 100644 --- a/src/server/injected/injectedScript.ts +++ b/src/server/injected/injectedScript.ts @@ -19,7 +19,7 @@ import { createCSSEngine } from './cssSelectorEngine'; import { SelectorEngine, SelectorRoot } from './selectorEngine'; import { createTextSelector } from './textSelectorEngine'; import { XPathEngine } from './xpathSelectorEngine'; -import { ParsedSelector, ParsedSelectorV1, parseSelector } from '../common/selectorParser'; +import { ParsedSelector, ParsedSelectorV1, parseSelector, selectorsV2Enabled, selectorsV2EngineNames } from '../common/selectorParser'; import { FatalDOMError } from '../common/domErrors'; import { SelectorEvaluatorImpl, SelectorEngine as SelectorEngineV2, QueryContext, isVisible, parentElementOrShadowHost } from './selectorEvaluator'; @@ -43,6 +43,7 @@ export type InjectedScriptPoll = { export class InjectedScript { private _enginesV1: Map; private _evaluator: SelectorEvaluatorImpl; + private _engineNames: Set; constructor(customEngines: { name: string, engine: SelectorEngine}[]) { this._enginesV1 = new Map(); @@ -67,10 +68,16 @@ export class InjectedScript { for (const { name, engine } of customEngines) wrapped.set(name, wrapV2(name, engine)); this._evaluator = new SelectorEvaluatorImpl(wrapped); + + this._engineNames = new Set(this._enginesV1.keys()); + if (selectorsV2Enabled()) { + for (const name of selectorsV2EngineNames()) + this._engineNames.add(name); + } } parseSelector(selector: string): ParsedSelector { - return parseSelector(selector); + return parseSelector(selector, this._engineNames); } querySelector(selector: ParsedSelector, root: Node): Element | undefined { diff --git a/src/server/selectors.ts b/src/server/selectors.ts index 4f08e2ce66..57b7565908 100644 --- a/src/server/selectors.ts +++ b/src/server/selectors.ts @@ -18,7 +18,7 @@ import * as dom from './dom'; import * as frames from './frames'; import * as js from './javascript'; import * as types from './types'; -import { ParsedSelector, parseSelector, selectorsV2Enabled } from './common/selectorParser'; +import { ParsedSelector, parseSelector, selectorsV2Enabled, selectorsV2EngineNames } from './common/selectorParser'; export type SelectorInfo = { parsed: ParsedSelector, @@ -29,6 +29,7 @@ export type SelectorInfo = { export class Selectors { readonly _builtinEngines: Set; readonly _engines: Map; + readonly _engineNames: Set; constructor() { // Note: keep in sync with SelectorEvaluator class. @@ -42,10 +43,11 @@ export class Selectors { 'data-test', 'data-test:light', ]); if (selectorsV2Enabled()) { - for (const name of ['not', 'is', 'where', 'has', 'scope', 'light', 'index', 'visible', 'matches-text', 'above', 'below', 'right-of', 'left-of', 'near', 'within']) + for (const name of selectorsV2EngineNames()) this._builtinEngines.add(name); } this._engines = new Map(); + this._engineNames = new Set(this._builtinEngines); } async register(name: string, source: string, contentScript: boolean = false): Promise { @@ -57,6 +59,7 @@ export class Selectors { if (this._engines.has(name)) throw new Error(`"${name}" selector engine has been already registered`); this._engines.set(name, { source, contentScript }); + this._engineNames.add(name); } async _query(frame: frames.Frame, selector: string, scope?: dom.ElementHandle): Promise | null> { @@ -119,11 +122,7 @@ export class Selectors { } _parseSelector(selector: string): SelectorInfo { - const parsed = parseSelector(selector); - for (const name of parsed.names) { - if (!this._builtinEngines.has(name) && !this._engines.has(name)) - throw new Error(`Unknown engine "${name}" while parsing selector ${selector}`); - } + const parsed = parseSelector(selector, this._engineNames); const needsMainWorld = parsed.names.some(name => { const custom = this._engines.get(name); return custom ? !custom.contentScript : false; diff --git a/test/css-parser.spec.ts b/test/css-parser.spec.ts index b9d59c40fc..501dbcf5b0 100644 --- a/test/css-parser.spec.ts +++ b/test/css-parser.spec.ts @@ -20,58 +20,62 @@ import * as path from 'path'; const { parseCSS, serializeSelector: serialize } = require(path.join(__dirname, '..', 'lib', 'server', 'common', 'cssParser')); +const parse = (selector: string) => { + return parseCSS(selector, new Set(['text', 'not', 'has', 'react', 'scope', 'right-of', 'scope', 'is'])).selector; +}; + it('should parse css', async () => { - expect(serialize(parseCSS('div').selector)).toBe('div'); - expect(serialize(parseCSS('div.class').selector)).toBe('div.class'); - expect(serialize(parseCSS('.class').selector)).toBe('.class'); - expect(serialize(parseCSS('#id').selector)).toBe('#id'); - expect(serialize(parseCSS('.class#id').selector)).toBe('.class#id'); - expect(serialize(parseCSS('div#id.class').selector)).toBe('div#id.class'); - expect(serialize(parseCSS('*').selector)).toBe('*'); - expect(serialize(parseCSS('*div').selector)).toBe('*div'); - expect(serialize(parseCSS('div[attr *= foo i]').selector)).toBe('div[attr *= foo i]'); - expect(serialize(parseCSS('div[attr~="Bar baz" ]').selector)).toBe('div[attr~="Bar baz" ]'); - expect(serialize(parseCSS(`div [ foo = 'bar' s]`).selector)).toBe(`div [ foo = "bar" s]`); + expect(serialize(parse('div'))).toBe('div'); + expect(serialize(parse('div.class'))).toBe('div.class'); + expect(serialize(parse('.class'))).toBe('.class'); + expect(serialize(parse('#id'))).toBe('#id'); + expect(serialize(parse('.class#id'))).toBe('.class#id'); + expect(serialize(parse('div#id.class'))).toBe('div#id.class'); + expect(serialize(parse('*'))).toBe('*'); + expect(serialize(parse('*div'))).toBe('*div'); + expect(serialize(parse('div[attr *= foo i]'))).toBe('div[attr *= foo i]'); + expect(serialize(parse('div[attr~="Bar baz" ]'))).toBe('div[attr~="Bar baz" ]'); + expect(serialize(parse(`div [ foo = 'bar' s]`))).toBe(`div [ foo = "bar" s]`); - expect(serialize(parseCSS(':hover').selector)).toBe(':hover'); - expect(serialize(parseCSS('div:hover').selector)).toBe('div:hover'); - expect(serialize(parseCSS('#id:active:hover').selector)).toBe('#id:active:hover'); - expect(serialize(parseCSS(':dir(ltr)').selector)).toBe(':dir(ltr)'); - expect(serialize(parseCSS('#foo-bar.cls:nth-child(3n + 10)').selector)).toBe('#foo-bar.cls:nth-child(3n + 10)'); - expect(serialize(parseCSS(':lang(en)').selector)).toBe(':lang(en)'); - expect(serialize(parseCSS('*:hover').selector)).toBe('*:hover'); + expect(serialize(parse(':hover'))).toBe(':hover'); + expect(serialize(parse('div:hover'))).toBe('div:hover'); + expect(serialize(parse('#id:active:hover'))).toBe('#id:active:hover'); + expect(serialize(parse(':dir(ltr)'))).toBe(':dir(ltr)'); + expect(serialize(parse('#foo-bar.cls:nth-child(3n + 10)'))).toBe('#foo-bar.cls:nth-child(3n + 10)'); + expect(serialize(parse(':lang(en)'))).toBe(':lang(en)'); + expect(serialize(parse('*:hover'))).toBe('*:hover'); - expect(serialize(parseCSS('div span').selector)).toBe('div span'); - expect(serialize(parseCSS('div>span').selector)).toBe('div > span'); - expect(serialize(parseCSS('div +span').selector)).toBe('div + span'); - expect(serialize(parseCSS('div~ span').selector)).toBe('div ~ span'); - expect(serialize(parseCSS('div >.class #id+ span').selector)).toBe('div > .class #id + span'); - expect(serialize(parseCSS('div>span+.class').selector)).toBe('div > span + .class'); + expect(serialize(parse('div span'))).toBe('div span'); + expect(serialize(parse('div>span'))).toBe('div > span'); + expect(serialize(parse('div +span'))).toBe('div + span'); + expect(serialize(parse('div~ span'))).toBe('div ~ span'); + expect(serialize(parse('div >.class #id+ span'))).toBe('div > .class #id + span'); + expect(serialize(parse('div>span+.class'))).toBe('div > span + .class'); - expect(serialize(parseCSS('div:not(span)').selector)).toBe('div:not(span)'); - expect(serialize(parseCSS(':not(span)#id').selector)).toBe('#id:not(span)'); - expect(serialize(parseCSS('div:not(span):hover').selector)).toBe('div:hover:not(span)'); - expect(serialize(parseCSS('div:has(span):hover').selector)).toBe('div:hover:has(span)'); - expect(serialize(parseCSS('div:right-of(span):hover').selector)).toBe('div:hover:right-of(span)'); - expect(serialize(parseCSS(':right-of(span):react(foobar)').selector)).toBe(':right-of(span):react(foobar)'); - expect(serialize(parseCSS('div:is(span):hover').selector)).toBe('div:hover:is(span)'); - expect(serialize(parseCSS('div:scope:hover').selector)).toBe('div:hover:scope()'); - expect(serialize(parseCSS('div:sCOpe:HOVER').selector)).toBe('div:HOVER:scope()'); - expect(serialize(parseCSS('div:NOT(span):hoVER').selector)).toBe('div:hoVER:not(span)'); + expect(serialize(parse('div:not(span)'))).toBe('div:not(span)'); + expect(serialize(parse(':not(span)#id'))).toBe('#id:not(span)'); + expect(serialize(parse('div:not(span):hover'))).toBe('div:hover:not(span)'); + expect(serialize(parse('div:has(span):hover'))).toBe('div:hover:has(span)'); + expect(serialize(parse('div:right-of(span):hover'))).toBe('div:hover:right-of(span)'); + expect(serialize(parse(':right-of(span):react(foobar)'))).toBe(':right-of(span):react(foobar)'); + expect(serialize(parse('div:is(span):hover'))).toBe('div:hover:is(span)'); + expect(serialize(parse('div:scope:hover'))).toBe('div:hover:scope()'); + expect(serialize(parse('div:sCOpe:HOVER'))).toBe('div:HOVER:scope()'); + expect(serialize(parse('div:NOT(span):hoVER'))).toBe('div:hoVER:not(span)'); - expect(serialize(parseCSS(':text("foo")').selector)).toBe(':text("foo")'); - expect(serialize(parseCSS(':text("*")').selector)).toBe(':text("*")'); - expect(serialize(parseCSS(':text(*)').selector)).toBe(':text(*)'); - expect(serialize(parseCSS(':text("foo", normalize-space)').selector)).toBe(':text("foo", normalize-space)'); - expect(serialize(parseCSS(':index(3, div span)').selector)).toBe(':index(3, div span)'); - expect(serialize(parseCSS(':is(foo, bar>baz.cls+:not(qux))').selector)).toBe(':is(foo, bar > baz.cls + :not(qux))'); + expect(serialize(parse(':text("foo")'))).toBe(':text("foo")'); + expect(serialize(parse(':text("*")'))).toBe(':text("*")'); + expect(serialize(parse(':text(*)'))).toBe(':text(*)'); + expect(serialize(parse(':text("foo", normalize-space)'))).toBe(':text("foo", normalize-space)'); + expect(serialize(parse(':index(3, div span)'))).toBe(':index(3, div span)'); + expect(serialize(parse(':is(foo, bar>baz.cls+:not(qux))'))).toBe(':is(foo, bar > baz.cls + :not(qux))'); }); it('should throw on malformed css', async () => { function expectError(selector: string) { let error = { message: '' }; try { - parseCSS(selector); + parse(selector); } catch (e) { error = e; }