Skip to content

Commit

Permalink
fix(selector): bring back v1 query logic (#4754)
Browse files Browse the repository at this point in the history 
It turned out that v1 query logic is not shimmable by v2 logic.
This change brings back v1 query logic for `>>` combinator.
  • Loading branch information
@dgozman
dgozman authored Dec 18, 2020
1 parent 9a0023c commit 5a1c9f1
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 175 deletions.
147 changes: 27 additions & 120 deletions src/server/common/selectorParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,144 +14,51 @@
* limitations under the License.
*/

import { CSSComplexSelector, CSSComplexSelectorList, CSSFunctionArgument, CSSSimpleSelector, parseCSS } from './cssParser';
import { CSSComplexSelectorList, parseCSS } from './cssParser';

export type ParsedSelectorV1 = {
parts: {
name: string,
body: string,
}[],
capture?: number,
};
export type ParsedSelectorPart = {
name: string,
body: string,
} | CSSComplexSelectorList;

export type ParsedSelector = {
v1?: ParsedSelectorV1,
v2?: CSSComplexSelectorList,
names: string[],
parts: ParsedSelectorPart[],
capture?: number,
};

export function selectorsV2Enabled() {
return true;
}

export function selectorsV2EngineNames() {
return ['not', 'is', 'where', 'has', 'scope', 'light', 'visible', 'text-matches', 'text-is'];
}
const customCSSNames = new Set(['not', 'is', 'where', 'has', 'scope', 'light', 'visible', 'text', 'text-matches', 'text-is']);

export function parseSelector(selector: string, customNames: Set<string>): ParsedSelector {
const v1 = parseSelectorV1(selector);
const names = new Set<string>();
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 {
v1,
names: Array.from(names),
};
}
export function parseSelector(selector: string): ParsedSelector {
const result = parseSelectorV1(selector);

const chain = (from: number, to: number, turnFirstTextIntoScope: boolean): CSSComplexSelector => {
const result: CSSComplexSelector = { simples: [] };
for (const part of v1.parts.slice(from, to)) {
let name = part.name;
let wrapInLight = false;
if (['css:light', 'xpath:light', 'text:light', 'id:light', 'data-testid:light', 'data-test-id:light', 'data-test:light'].includes(name)) {
wrapInLight = true;
name = name.substring(0, name.indexOf(':'));
if (selectorsV2Enabled()) {
result.parts = result.parts.map(part => {
if (Array.isArray(part))
return part;
if (part.name === 'css' || part.name === 'css:light') {
if (part.name === 'css:light')
part.body = ':light(' + part.body + ')';
const parsedCSS = parseCSS(part.body, customCSSNames);
return parsedCSS.selector;
}
if (name === 'css') {
const parsed = parseCSS(part.body, customNames);
parsed.names.forEach(name => names.add(name));
if (wrapInLight || parsed.selector.length > 1) {
let simple = callWith('is', parsed.selector);
if (wrapInLight)
simple = callWith('light', [simpleToComplex(simple)]);
result.simples.push({ selector: simple, combinator: '' });
} else {
result.simples.push(...parsed.selector[0].simples);
}
} else if (name === 'text') {
let simple = textSelectorToSimple(part.body);
if (turnFirstTextIntoScope)
simple.functions.push({ name: 'is', args: [ simpleToComplex(callWith('scope', [])), simpleToComplex({ css: '*', functions: [] }) ]});
if (result.simples.length)
result.simples[result.simples.length - 1].combinator = '>=';
if (wrapInLight)
simple = callWith('light', [simpleToComplex(simple)]);
result.simples.push({ selector: simple, combinator: '' });
} else {
let simple = callWith(name, [part.body]);
if (wrapInLight)
simple = callWith('light', [simpleToComplex(simple)]);
result.simples.push({ selector: simple, combinator: '' });
}
if (name !== 'text')
turnFirstTextIntoScope = false;
}
return result;
};

const capture = v1.capture === undefined ? v1.parts.length - 1 : v1.capture;
const result = chain(0, capture + 1, false);
if (capture + 1 < v1.parts.length) {
const has = chain(capture + 1, v1.parts.length, true);
const last = result.simples[result.simples.length - 1];
last.selector.functions.push({ name: 'has', args: [has] });
return part;
});
}
return { v2: [result], names: Array.from(names) };
}

function callWith(name: string, args: CSSFunctionArgument[]): CSSSimpleSelector {
return { functions: [{ name, args }] };
}

function simpleToComplex(simple: CSSSimpleSelector): CSSComplexSelector {
return { simples: [{ selector: simple, combinator: '' }]};
}

function textSelectorToSimple(selector: string): CSSSimpleSelector {
function unescape(s: string): string {
if (!s.includes('\\'))
return s;
const r: string[] = [];
let i = 0;
while (i < s.length) {
if (s[i] === '\\' && i + 1 < s.length)
i++;
r.push(s[i++]);
}
return r.join('');
}

function escapeRegExp(s: string) {
return s.replace(/[.*+\?^${}()|[\]\\]/g, '\\$&').replace(/-/g, '\\x2d');
}

let functionName = 'text-matches';
let args: string[];
if (selector.length > 1 && selector[0] === '"' && selector[selector.length - 1] === '"') {
args = ['^' + escapeRegExp(unescape(selector.substring(1, selector.length - 1))) + '$'];
} else if (selector.length > 1 && selector[0] === "'" && selector[selector.length - 1] === "'") {
args = ['^' + escapeRegExp(unescape(selector.substring(1, selector.length - 1))) + '$'];
} else if (selector[0] === '/' && selector.lastIndexOf('/') > 0) {
const lastSlash = selector.lastIndexOf('/');
args = [selector.substring(1, lastSlash), selector.substring(lastSlash + 1)];
} else {
functionName = 'text';
args = [selector];
}
return callWith(functionName, args);
return {
parts: result.parts,
capture: result.capture,
};
}

function parseSelectorV1(selector: string): ParsedSelectorV1 {
function parseSelectorV1(selector: string): ParsedSelector {
let index = 0;
let quote: string | undefined;
let start = 0;
const result: ParsedSelectorV1 = { parts: [] };
const result: ParsedSelector = { parts: [] };
const append = () => {
const part = selector.substring(start, index).trim();
const eqIndex = part.indexOf('=');
Expand Down
74 changes: 32 additions & 42 deletions src/server/injected/injectedScript.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
*/

import { createAttributeEngine } from './attributeSelectorEngine';
import { createCSSEngine } from './cssSelectorEngine';
import { SelectorEngine, SelectorRoot } from './selectorEngine';
import { createTextSelector } from './textSelectorEngine';
import { XPathEngine } from './xpathSelectorEngine';
import { ParsedSelector, ParsedSelectorV1, parseSelector, selectorsV2Enabled, selectorsV2EngineNames } from '../common/selectorParser';
import { ParsedSelector, ParsedSelectorPart, parseSelector } from '../common/selectorParser';
import { FatalDOMError } from '../common/domErrors';
import { SelectorEvaluatorImpl, SelectorEngine as SelectorEngineV2, QueryContext, isVisible, parentElementOrShadowHost } from './selectorEvaluator';
import { SelectorEvaluatorImpl, isVisible, parentElementOrShadowHost } from './selectorEvaluator';
import { createCSSEngine } from './cssSelectorEngine';

type Predicate<T> = (progress: InjectedScriptProgress, continuePolling: symbol) => T | symbol;

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

constructor(customEngines: { name: string, engine: SelectorEngine}[]) {
this._enginesV1 = new Map();
Expand All @@ -64,37 +63,32 @@ export class InjectedScript {
for (const { name, engine } of customEngines)
this._enginesV1.set(name, engine);

const wrapped = new Map<string, SelectorEngineV2>();
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);
}
// No custom engines in V2 for now.
this._evaluator = new SelectorEvaluatorImpl(new Map());
}

parseSelector(selector: string): ParsedSelector {
return parseSelector(selector, this._engineNames);
const result = parseSelector(selector);
for (const part of result.parts) {
if (!Array.isArray(part) && !this._enginesV1.has(part.name))
throw new Error(`Unknown engine "${part.name}" while parsing selector ${selector}`);
}
return result;
}

querySelector(selector: ParsedSelector, root: Node): Element | undefined {
if (!(root as any)['querySelector'])
throw new Error('Node is not queryable.');
if (selector.v1)
return this._querySelectorRecursivelyV1(root as SelectorRoot, selector.v1, 0);
return this._evaluator.evaluate({ scope: root as Document | Element, pierceShadow: true }, selector.v2!)[0];
return this._querySelectorRecursively(root as SelectorRoot, selector, 0);
}

private _querySelectorRecursivelyV1(root: SelectorRoot, selector: ParsedSelectorV1, index: number): Element | undefined {
private _querySelectorRecursively(root: SelectorRoot, selector: ParsedSelector, index: number): Element | undefined {
const current = selector.parts[index];
if (index === selector.parts.length - 1)
return this._enginesV1.get(current.name)!.query(root, current.body);
const all = this._enginesV1.get(current.name)!.queryAll(root, current.body);
return this._queryEngine(current, root);
const all = this._queryEngineAll(current, root);
for (const next of all) {
const result = this._querySelectorRecursivelyV1(next, selector, index + 1);
const result = this._querySelectorRecursively(next, selector, index + 1);
if (result)
return selector.capture === index ? next : result;
}
Expand All @@ -103,22 +97,16 @@ export class InjectedScript {
querySelectorAll(selector: ParsedSelector, root: Node): Element[] {
if (!(root as any)['querySelectorAll'])
throw new Error('Node is not queryable.');
if (selector.v1)
return this._querySelectorAllV1(selector.v1, root as SelectorRoot);
return this._evaluator.evaluate({ scope: root as Document | Element, pierceShadow: true }, selector.v2!);
}

private _querySelectorAllV1(selector: ParsedSelectorV1, root: SelectorRoot): Element[] {
const capture = selector.capture === undefined ? selector.parts.length - 1 : selector.capture;
// Query all elements up to the capture.
const partsToQuerAll = selector.parts.slice(0, capture + 1);
const partsToQueryAll = selector.parts.slice(0, capture + 1);
// Check they have a descendant matching everything after the capture.
const partsToCheckOne = selector.parts.slice(capture + 1);
let set = new Set<SelectorRoot>([ root as SelectorRoot ]);
for (const { name, body } of partsToQuerAll) {
for (const part of partsToQueryAll) {
const newSet = new Set<Element>();
for (const prev of set) {
for (const next of this._enginesV1.get(name)!.queryAll(prev, body)) {
for (const next of this._queryEngineAll(part, prev)) {
if (newSet.has(next))
continue;
newSet.add(next);
Expand All @@ -130,7 +118,19 @@ export class InjectedScript {
if (!partsToCheckOne.length)
return candidates;
const partial = { parts: partsToCheckOne };
return candidates.filter(e => !!this._querySelectorRecursivelyV1(e, partial, 0));
return candidates.filter(e => !!this._querySelectorRecursively(e, partial, 0));
}

private _queryEngine(part: ParsedSelectorPart, root: SelectorRoot): Element | undefined {
if (Array.isArray(part))
return this._evaluator.evaluate({ scope: root as Document | Element, pierceShadow: true }, part)[0];
return this._enginesV1.get(part.name)!.query(root, part.body);
}

private _queryEngineAll(part: ParsedSelectorPart, root: SelectorRoot): Element[] {
if (Array.isArray(part))
return this._evaluator.evaluate({ scope: root as Document | Element, pierceShadow: true }, part);
return this._enginesV1.get(part.name)!.queryAll(root, part.body);
}

extend(source: string, params: any): any {
Expand Down Expand Up @@ -667,16 +667,6 @@ export class InjectedScript {
}
}

function wrapV2(name: string, engine: SelectorEngine): SelectorEngineV2 {
return {
query(context: QueryContext, args: string[]): Element[] {
if (args.length !== 1 || typeof args[0] !== 'string')
throw new Error(`engine "${name}" expects a single string`);
return engine.queryAll(context.scope, args[0]);
}
};
}

const autoClosingTags = new Set(['AREA', 'BASE', 'BR', 'COL', 'COMMAND', 'EMBED', 'HR', 'IMG', 'INPUT', 'KEYGEN', 'LINK', 'MENUITEM', 'META', 'PARAM', 'SOURCE', 'TRACK', 'WBR']);
const booleanAttributes = new Set(['checked', 'selected', 'disabled', 'readonly', 'multiple']);

Expand Down
25 changes: 12 additions & 13 deletions src/server/selectors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, selectorsV2EngineNames } from './common/selectorParser';
import { ParsedSelector, parseSelector } from './common/selectorParser';

export type SelectorInfo = {
parsed: ParsedSelector,
Expand All @@ -29,7 +29,6 @@ export type SelectorInfo = {
export class Selectors {
readonly _builtinEngines: Set<string>;
readonly _engines: Map<string, { source: string, contentScript: boolean }>;
readonly _engineNames: Set<string>;

constructor() {
// Note: keep in sync with SelectorEvaluator class.
Expand All @@ -42,12 +41,7 @@ export class Selectors {
'data-test-id', 'data-test-id:light',
'data-test', 'data-test:light',
]);
if (selectorsV2Enabled()) {
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<void> {
Expand All @@ -59,7 +53,6 @@ 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<dom.ElementHandle<Element> | null> {
Expand Down Expand Up @@ -122,11 +115,17 @@ export class Selectors {
}

_parseSelector(selector: string): SelectorInfo {
const parsed = parseSelector(selector, this._engineNames);
const needsMainWorld = parsed.names.some(name => {
const custom = this._engines.get(name);
return custom ? !custom.contentScript : false;
});
const parsed = parseSelector(selector);
let needsMainWorld = false;
for (const part of parsed.parts) {
if (!Array.isArray(part)) {
const custom = this._engines.get(part.name);
if (!custom && !this._builtinEngines.has(part.name))
throw new Error(`Unknown engine "${part.name}" while parsing selector ${selector}`);
if (custom && !custom.contentScript)
needsMainWorld = true;
}
}
return {
parsed,
selector,
Expand Down
1 change: 1 addition & 0 deletions test/selectors-css.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ it('should work with spaces in :nth-child and :not', async ({page, server}) => {
expect(await page.$$eval(`css=div > :not(span)`, els => els.length)).toBe(2);
expect(await page.$$eval(`css=body :not(span, div)`, els => els.length)).toBe(1);
expect(await page.$$eval(`css=span, section:not(span, div)`, els => els.length)).toBe(5);
expect(await page.$$eval(`span:nth-child(23n+ 2) >> xpath=.`, els => els.length)).toBe(1);
});

it('should work with :is', async ({page, server}) => {
Expand Down
5 changes: 5 additions & 0 deletions test/selectors-misc.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,8 @@ it('should work with proximity selectors', test => {
expect(await page.$$eval('div:near(#id7)', els => els.map(e => e.id).join(','))).toBe('id0,id3,id4,id5,id6');
expect(await page.$$eval('div:near(#id0)', els => els.map(e => e.id).join(','))).toBe('id1,id2,id3,id4,id5,id7,id8,id9');
});

it('should escape the scope with >>', async ({ page }) => {
await page.setContent(`<div><label>Test</label><input id='myinput'></div>`);
expect(await page.$eval(`label >> xpath=.. >> input`, e => e.id)).toBe('myinput');
});

0 comments on commit 5a1c9f1

Please sign in to comment.