Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

feat(codegen): make selector generator strict (#11856)

Browse source 
This is required to migrate to locators.
This commit is contained in:
Dmitry Gozman 2022-02-04 07:34:23 -08:00 committed by GitHub
parent d9c82b8784
commit c45dacc834
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 43 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
packages/playwright-core/src/server/injected/injectedScript.ts
View file

@ -848,7 +848,7 @@ export class InjectedScript {
strictModeViolationError(selector: ParsedSelector, matches: Element[]): Error {
const infos = matches.slice(0, 10).map(m => ({
preview: this.previewNode(m),
selector: generateSelector(this, m).selector
selector: generateSelector(this, m, true).selector
}));
const lines = infos.map((info, i) => `\n ${i + 1}) ${info.preview} aka playwright.$("${info.selector}")`);
if (infos.length < matches.length)

54
packages/playwright-core/src/server/injected/selectorGenerator.ts
View file

@ -25,6 +25,7 @@ type SelectorToken = {
const cacheAllowText = new Map<Element, SelectorToken[] | null>();
const cacheDisallowText = new Map<Element, SelectorToken[] | null>();
const kNthScore = 1000;
export function querySelector(injectedScript: InjectedScript, selector: string, ownerDocument: Document): { selector: string, elements: Element[] } {
try {
@ -41,12 +42,12 @@ export function querySelector(injectedScript: InjectedScript, selector: string,
}
}
export function generateSelector(injectedScript: InjectedScript, targetElement: Element): { selector: string, elements: Element[] } {
export function generateSelector(injectedScript: InjectedScript, targetElement: Element, strict: boolean): { selector: string, elements: Element[] } {
injectedScript._evaluator.begin();
try {
targetElement = targetElement.closest('button,select,input,[role=button],[role=checkbox],[role=radio]') || targetElement;
const targetTokens = generateSelectorFor(injectedScript, targetElement);
const bestTokens = targetTokens || [cssFallback(injectedScript, targetElement)];
const targetTokens = generateSelectorFor(injectedScript, targetElement, strict);
const bestTokens = targetTokens || cssFallback(injectedScript, targetElement, strict);
const selector = joinTokens(bestTokens);
const parsedSelector = injectedScript.parseSelector(selector);
return {
@ -65,7 +66,7 @@ function filterRegexTokens(textCandidates: SelectorToken[][]): SelectorToken[][]
return textCandidates.filter(c => c[0].selector[0] !== '/');
}
function generateSelectorFor(injectedScript: InjectedScript, targetElement: Element): SelectorToken[] | null {
function generateSelectorFor(injectedScript: InjectedScript, targetElement: Element, strict: boolean): SelectorToken[] | null {
if (targetElement.ownerDocument.documentElement === targetElement)
return [{ engine: 'css', selector: 'html', score: 1 }];
@ -80,7 +81,7 @@ function generateSelectorFor(injectedScript: InjectedScript, targetElement: Elem
const noTextCandidates = buildCandidates(injectedScript, element).map(token => [token]);
// First check all text and non-text candidates for the element.
let result = chooseFirstSelector(injectedScript, targetElement.ownerDocument, element, [...textCandidates, ...noTextCandidates], allowNthMatch);
let result = chooseFirstSelector(injectedScript, targetElement.ownerDocument, element, [...textCandidates, ...noTextCandidates], allowNthMatch, strict);
// Do not use regex for chained selectors (for performance).
textCandidates = filterRegexTokens(textCandidates);
@ -110,7 +111,7 @@ function generateSelectorFor(injectedScript: InjectedScript, targetElement: Elem
if (result && combineScores([...parentTokens, ...bestPossibleInParent]) >= combineScores(result))
continue;
// Update the best candidate that finds "element" in the "parent".
bestPossibleInParent = chooseFirstSelector(injectedScript, parent, element, candidates, allowNthMatch);
bestPossibleInParent = chooseFirstSelector(injectedScript, parent, element, candidates, allowNthMatch, strict);
if (!bestPossibleInParent)
return;
const combined = [...parentTokens, ...bestPossibleInParent];
@ -214,7 +215,7 @@ function makeSelectorForId(id: string) {
return /^[a-zA-Z][a-zA-Z0-9\-\_]+$/.test(id) ? '#' + id : `[id="${cssEscape(id)}"]`;
}
function cssFallback(injectedScript: InjectedScript, targetElement: Element): SelectorToken {
function cssFallback(injectedScript: InjectedScript, targetElement: Element, strict: boolean): SelectorToken[] {
const kFallbackScore = 10000000;
const root: Node = targetElement.ownerDocument;
const tokens: string[] = [];
@ -229,6 +230,18 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element): Se
return node === targetElement ? selector : undefined;
}
function makeStrict(selector: string): SelectorToken[] {
const token = { engine: 'css', selector, score: kFallbackScore };
if (!strict)
return [token];
const parsedSelector = injectedScript.parseSelector(selector);
const elements = injectedScript.querySelectorAll(parsedSelector, targetElement.ownerDocument);
if (elements.length === 1)
return [token];
const nth = { engine: 'nth', selector: String(elements.indexOf(targetElement)), score: kNthScore };
return [token, nth];
}
for (let element: Element | null = targetElement; element && element !== root; element = parentElementOrShadowHost(element)) {
const nodeName = element.nodeName.toLowerCase();
@ -238,7 +251,7 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element): Se
const token = makeSelectorForId(element.id);
const selector = uniqueCSSSelector(token);
if (selector)
return { engine: 'css', selector, score: kFallbackScore };
return makeStrict(selector);
bestTokenForLevel = token;
}
@ -250,7 +263,7 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element): Se
const token = '.' + classes.slice(0, i + 1).join('.');
const selector = uniqueCSSSelector(token);
if (selector)
return { engine: 'css', selector, score: kFallbackScore };
return makeStrict(selector);
// Even if not unique, does this subset of classes uniquely identify node as a child?
if (!bestTokenForLevel && parent) {
const sameClassSiblings = parent.querySelectorAll(token);
@ -266,7 +279,7 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element): Se
const token = sameTagSiblings.indexOf(element) === 0 ? cssEscape(nodeName) : `${cssEscape(nodeName)}:nth-child(${1 + siblings.indexOf(element)})`;
const selector = uniqueCSSSelector(token);
if (selector)
return { engine: 'css', selector, score: kFallbackScore };
return makeStrict(selector);
if (!bestTokenForLevel)
bestTokenForLevel = token;
} else if (!bestTokenForLevel) {
@ -274,7 +287,7 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element): Se
}
tokens.unshift(bestTokenForLevel);
}
return { engine: 'css', selector: uniqueCSSSelector()!, score: kFallbackScore };
return makeStrict(uniqueCSSSelector()!);
}
function escapeForRegex(text: string): string {
@ -307,7 +320,7 @@ function combineScores(tokens: SelectorToken[]): number {
return score;
}
function chooseFirstSelector(injectedScript: InjectedScript, scope: Element | Document, targetElement: Element, selectors: SelectorToken[][], allowNthMatch: boolean): SelectorToken[] | null {
function chooseFirstSelector(injectedScript: InjectedScript, scope: Element | Document, targetElement: Element, selectors: SelectorToken[][], allowNthMatch: boolean, strict: boolean): SelectorToken[] | null {
const joined = selectors.map(tokens => ({ tokens, score: combineScores(tokens) }));
joined.sort((a, b) => a.score - b.score);
@ -315,26 +328,19 @@ function chooseFirstSelector(injectedScript: InjectedScript, scope: Element | Do
for (const { tokens } of joined) {
const parsedSelector = injectedScript.parseSelector(joinTokens(tokens));
const result = injectedScript.querySelectorAll(parsedSelector, scope);
const isStrictEnough = !strict || result.length === 1;
const index = result.indexOf(targetElement);
if (index === 0) {
if (index === 0 && isStrictEnough) {
// We are the first match - found the best selector.
return tokens;
}
// Otherwise, perhaps we can get nth-match?
// Otherwise, perhaps we can use nth=?
if (!allowNthMatch || bestWithIndex || index === -1 || result.length > 5)
continue;
// To use nth-match, we must convert everything to css.
const allCss = tokens.map(token => {
if (token.engine !== 'text')
return token;
if (token.selector.startsWith('/') && token.selector.endsWith('/'))
return { engine: 'css', selector: `:text-matches("${token.selector.substring(1, token.selector.length - 1)}")`, score: token.score };
return { engine: 'css', selector: `:text("${token.selector}")`, score: token.score };
});
const combined = joinTokens(allCss);
bestWithIndex = [{ engine: 'css', selector: `:nth-match(${combined}, ${index + 1})`, score: combineScores(allCss) + 1000 }];
const nth: SelectorToken = { engine: 'nth', selector: String(index), score: kNthScore };
bestWithIndex = [...tokens, nth];
}
return bestWithIndex;
}

2
packages/playwright-core/src/server/supplements/injected/consoleApi.ts
View file

@ -104,7 +104,7 @@ export class ConsoleAPI {
private _selector(element: Element) {
if (!(element instanceof Element))
throw new Error(`Usage: playwright.selector(element).`);
return generateSelector(this._injectedScript, element).selector;
return generateSelector(this._injectedScript, element, true).selector;
}
private _resume() {

4
packages/playwright-core/src/server/supplements/injected/recorder.ts
View file

@ -237,7 +237,7 @@ export class Recorder {
private _onFocus() {
const activeElement = this._deepActiveElement(document);
const result = activeElement ? generateSelector(this._injectedScript, activeElement) : null;
const result = activeElement ? generateSelector(this._injectedScript, activeElement, true) : null;
this._activeModel = result && result.selector ? result : null;
if (this._params.isUnderTest)
console.error('Highlight updated for test: ' + (result ? result.selector : null)); // eslint-disable-line no-console
@ -250,7 +250,7 @@ export class Recorder {
return;
}
const hoveredElement = this._hoveredElement;
const { selector, elements } = generateSelector(this._injectedScript, hoveredElement);
const { selector, elements } = generateSelector(this._injectedScript, hoveredElement, true);
if ((this._hoveredModel && this._hoveredModel.selector === selector) || this._hoveredElement !== hoveredElement)
return;
this._hoveredModel = selector ? { selector, elements } : null;

4
tests/page/page-strict.spec.ts
View file

@ -34,7 +34,7 @@ it('should fail page.fill in strict mode', async ({ page }) => {
await page.setContent(`<input></input><div><input></input></div>`);
const error = await page.fill('input', 'text', { strict: true }).catch(e => e);
expect(error.message).toContain('strict mode violation');
expect(error.message).toContain('1) <input/> aka playwright.$("input")');
expect(error.message).toContain('1) <input/> aka playwright.$("input >> nth=0")');
expect(error.message).toContain('2) <input/> aka playwright.$("div input")');
});
@ -54,6 +54,6 @@ it('should fail page.dispatchEvent in strict mode', async ({ page }) => {
await page.setContent(`<span></span><div><span></span></div>`);
const error = await page.dispatchEvent('span', 'click', {}, { strict: true }).catch(e => e);
expect(error.message).toContain('strict mode violation');
expect(error.message).toContain('1) <span></span> aka playwright.$("span")');
expect(error.message).toContain('1) <span></span> aka playwright.$("span >> nth=0")');
expect(error.message).toContain('2) <span></span> aka playwright.$("div span")');
});

14
tests/selector-generator.spec.ts
View file

@ -73,12 +73,12 @@ it.describe('selector generator', () => {
<select><option>foo</option></select>
<select mark=1><option>bar</option></select>
`);
expect(await generate(page, '[mark="1"]')).toBe(':nth-match(select, 2)');
expect(await generate(page, '[mark="1"]')).toBe('select >> nth=1');
});
it('should use ordinal for identical nodes', async ({ page }) => {
await page.setContent(`<div>Text</div><div>Text</div><div mark=1>Text</div><div>Text</div>`);
expect(await generate(page, 'div[mark="1"]')).toBe(`:nth-match(:text("Text"), 3)`);
expect(await generate(page, 'div[mark="1"]')).toBe(`text=Text >> nth=2`);
});
it('should prefer data-testid', async ({ page }) => {
@ -94,7 +94,7 @@ it.describe('selector generator', () => {
<div data-testid=a>
Text
</div>`);
expect(await generate(page, 'div[mark="1"]')).toBe('[data-testid="a"]');
expect(await generate(page, 'div[mark="1"]')).toBe('[data-testid="a"] >> nth=0');
});
it('should handle second non-unique data-testid', async ({ page }) => {
@ -105,7 +105,7 @@ it.describe('selector generator', () => {
<div data-testid=a mark=1>
Text
</div>`);
expect(await generate(page, 'div[mark="1"]')).toBe(`:nth-match([data-testid="a"], 2)`);
expect(await generate(page, 'div[mark="1"]')).toBe(`[data-testid="a"] >> nth=1`);
});
it('should use readable id', async ({ page }) => {
@ -121,7 +121,7 @@ it.describe('selector generator', () => {
<div></div>
<div id=aAbBcCdDeE mark=1></div>
`);
expect(await generate(page, 'div[mark="1"]')).toBe(`:nth-match(div, 2)`);
expect(await generate(page, 'div[mark="1"]')).toBe(`div >> nth=1`);
});
it('should use has-text', async ({ page }) => {
@ -195,7 +195,7 @@ it.describe('selector generator', () => {
<input value="two" mark="1">
<input value="three">
`);
expect(await generate(page, 'input[mark="1"]')).toBe(':nth-match(input, 2)');
expect(await generate(page, 'input[mark="1"]')).toBe('input >> nth=1');
});
it.describe('should prioritise input element attributes correctly', () => {
@ -249,7 +249,7 @@ it.describe('selector generator', () => {
input2.setAttribute('value', 'foo');
shadowRoot2.appendChild(input2);
});
expect(await generate(page, 'input[value=foo]')).toBe(':nth-match(input, 3)');
expect(await generate(page, 'input[value=foo]')).toBe('input >> nth=2');
});
it('should work in dynamic iframes without navigation', async ({ page }) => {