From 51fe84922cc036ebb4dbf1aa110570331e57b2ea Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 7 May 2020 13:36:54 -0700 Subject: [PATCH] fix(css selector): support comma-separated selector lists (#2120) --- src/injected/cssSelectorEngine.ts | 137 +++++++++++++++++++++--------- test/assets/deep-shadow.html | 1 + test/queryselector.spec.js | 34 ++++++++ tsconfig.json | 2 +- 4 files changed, 132 insertions(+), 42 deletions(-) diff --git a/src/injected/cssSelectorEngine.ts b/src/injected/cssSelectorEngine.ts index bebc7e9a16..c469fd0009 100644 --- a/src/injected/cssSelectorEngine.ts +++ b/src/injected/cssSelectorEngine.ts @@ -87,11 +87,13 @@ export function createCSSEngine(shadow: boolean): SelectorEngine { // return simple; // if (!shadow) // return; - const parts = split(selector); - if (!parts.length) + const selectors = split(selector); + // Note: we do not just merge results produced by each selector, as that + // will not return them in the tree traversal order, but rather in the selectors + // matching order. + if (!selectors.length) return; - parts.reverse(); - return queryShadowInternal(root, root, parts, shadow); + return queryShadowInternal(root, root, selectors, shadow); }, queryAll(root: SelectorRoot, selector: string): Element[] { @@ -99,11 +101,12 @@ export function createCSSEngine(shadow: boolean): SelectorEngine { // if (!shadow) // return Array.from(root.querySelectorAll(selector)); const result: Element[] = []; - const parts = split(selector); - if (parts.length) { - parts.reverse(); - queryShadowAllInternal(root, root, parts, shadow, result); - } + const selectors = split(selector); + // Note: we do not just merge results produced by each selector, as that + // will not return them in the tree traversal order, but rather in the selectors + // matching order. + if (selectors.length) + queryShadowAllInternal(root, root, selectors, shadow, result); return result; } }; @@ -111,49 +114,89 @@ export function createCSSEngine(shadow: boolean): SelectorEngine { return engine; } -function queryShadowInternal(boundary: SelectorRoot, root: SelectorRoot, parts: string[], shadow: boolean): Element | undefined { - const matching = root.querySelectorAll(parts[0]); - for (let i = 0; i < matching.length; i++) { - const element = matching[i]; - if (parts.length === 1 || matches(element, parts, boundary)) - return element; +function queryShadowInternal(boundary: SelectorRoot, root: SelectorRoot, selectors: string[][], shadow: boolean): Element | undefined { + let elements: NodeListOf | undefined; + if (selectors.length === 1) { + // Fast path for a single selector - query only matching elements, not all. + const parts = selectors[0]; + const matching = root.querySelectorAll(parts[0]); + for (const element of matching) { + // If there is a single part, there are no ancestors to match. + if (parts.length === 1 || ancestorsMatch(element, parts, boundary)) + return element; + } + } else { + // Multiple selectors: visit each element in tree-traversal order and check whether it matches. + elements = root.querySelectorAll('*'); + for (const element of elements) { + for (const parts of selectors) { + if (!element.matches(parts[0])) + continue; + // If there is a single part, there are no ancestors to match. + if (parts.length === 1 || ancestorsMatch(element, parts, boundary)) + return element; + } + } } + + // Visit shadow dom after the light dom to preserve the tree-traversal order. if (!shadow) return; if ((root as Element).shadowRoot) { - const child = queryShadowInternal(boundary, (root as Element).shadowRoot!, parts, shadow); + const child = queryShadowInternal(boundary, (root as Element).shadowRoot!, selectors, shadow); if (child) return child; } - const elements = root.querySelectorAll('*'); - for (let i = 0; i < elements.length; i++) { - const element = elements[i]; + if (!elements) + elements = root.querySelectorAll('*'); + for (const element of elements) { if (element.shadowRoot) { - const child = queryShadowInternal(boundary, element.shadowRoot, parts, shadow); + const child = queryShadowInternal(boundary, element.shadowRoot, selectors, shadow); if (child) return child; } } } -function queryShadowAllInternal(boundary: SelectorRoot, root: SelectorRoot, parts: string[], shadow: boolean, result: Element[]) { - const matching = root.querySelectorAll(parts[0]); - for (let i = 0; i < matching.length; i++) { - const element = matching[i]; - if (parts.length === 1 || matches(element, parts, boundary)) - result.push(element); +function queryShadowAllInternal(boundary: SelectorRoot, root: SelectorRoot, selectors: string[][], shadow: boolean, result: Element[]) { + let elements: NodeListOf | undefined; + if (selectors.length === 1) { + // Fast path for a single selector - query only matching elements, not all. + const parts = selectors[0]; + const matching = root.querySelectorAll(parts[0]); + for (const element of matching) { + // If there is a single part, there are no ancestors to match. + if (parts.length === 1 || ancestorsMatch(element, parts, boundary)) + result.push(element); + } + } else { + // Multiple selectors: visit each element in tree-traversal order and check whether it matches. + elements = root.querySelectorAll('*'); + for (const element of elements) { + for (const parts of selectors) { + if (!element.matches(parts[0])) + continue; + // If there is a single part, there are no ancestors to match. + if (parts.length === 1 || ancestorsMatch(element, parts, boundary)) + result.push(element); + } + } } - if (shadow && (root as Element).shadowRoot) - queryShadowAllInternal(boundary, (root as Element).shadowRoot!, parts, shadow, result); - const elements = root.querySelectorAll('*'); - for (let i = 0; i < elements.length; i++) { - const element = elements[i]; - if (shadow && element.shadowRoot) - queryShadowAllInternal(boundary, element.shadowRoot, parts, shadow, result); + + // Visit shadow dom after the light dom to preserve the tree-traversal order. + if (!shadow) + return; + if ((root as Element).shadowRoot) + queryShadowAllInternal(boundary, (root as Element).shadowRoot!, selectors, shadow, result); + if (!elements) + elements = root.querySelectorAll('*'); + for (const element of elements) { + if (element.shadowRoot) + queryShadowAllInternal(boundary, element.shadowRoot, selectors, shadow, result); } } -function matches(element: Element | undefined, parts: string[], boundary: SelectorRoot): boolean { +function ancestorsMatch(element: Element | undefined, parts: string[], boundary: SelectorRoot): boolean { let i = 1; while (i < parts.length && (element = parentElementOrShadowHost(element!)) && element !== boundary) { if (element.matches(parts[i])) @@ -171,16 +214,24 @@ function parentElementOrShadowHost(element: Element): Element | undefined { return (element.parentNode as ShadowRoot).host; } -function split(selector: string): string[] { +// Splits the string into separate selectors by comma, and then each selector by the descendant combinator (space). +// Parts of each selector are reversed, so that the first one matches the target element. +function split(selector: string): string[][] { let index = 0; let quote: string | undefined; let start = 0; let space: 'none' | 'before' | 'after' = 'none'; - const result: string[] = []; - const append = () => { + const result: string[][] = []; + let current: string[] = []; + const appendToCurrent = () => { const part = selector.substring(start, index).trim(); if (part.length) - result.push(part); + current.push(part); + }; + const appendToResult = () => { + appendToCurrent(); + result.push(current); + current = []; }; while (index < selector.length) { const c = selector[index]; @@ -193,7 +244,7 @@ function split(selector: string): string[] { if (c === '>' || c === '+' || c === '~') { space = 'after'; } else { - append(); + appendToCurrent(); start = index; space = 'none'; } @@ -208,13 +259,17 @@ function split(selector: string): string[] { } else if (c === '\'' || c === '"') { quote = c; index++; + } else if (!quote && c === ',') { + appendToResult(); + index++; + start = index; } else { index++; } } } - append(); - return result; + appendToResult(); + return result.filter(parts => !!parts.length).map(parts => parts.reverse()); } function test(engine: SelectorEngine) { diff --git a/test/assets/deep-shadow.html b/test/assets/deep-shadow.html index 761ecb454a..891b0730fa 100644 --- a/test/assets/deep-shadow.html +++ b/test/assets/deep-shadow.html @@ -4,6 +4,7 @@ window.addEventListener('DOMContentLoaded', () => { document.body.appendChild(outer); const root1 = document.createElement('div'); + root1.setAttribute('id', 'root1'); outer.appendChild(root1); const shadowRoot1 = root1.attachShadow({mode: 'open'}); const span1 = document.createElement('span'); diff --git a/test/queryselector.spec.js b/test/queryselector.spec.js index 1acc6014f9..bf4cc28187 100644 --- a/test/queryselector.spec.js +++ b/test/queryselector.spec.js @@ -721,6 +721,40 @@ describe('css selector', () => { expect(await root3.$eval(`css=[attr*="value"]`, e => e.textContent)).toBe('Hello from root3 #2'); expect(await root3.$(`css:light=[attr*="value"]`)).toBe(null); }); + + it('should work with comma separated list', async({page, server}) => { + await page.goto(server.PREFIX + '/deep-shadow.html'); + expect(await page.$$eval(`css=span,section #root1`, els => els.length)).toBe(5); + expect(await page.$$eval(`css=section #root1, div span`, els => els.length)).toBe(5); + expect(await page.$eval(`css=doesnotexist , section #root1`, e => e.id)).toBe('root1'); + expect(await page.$$eval(`css=doesnotexist ,section #root1`, els => els.length)).toBe(1); + expect(await page.$$eval(`css=span,div span`, els => els.length)).toBe(4); + expect(await page.$$eval(`css=span,div span`, els => els.length)).toBe(4); + expect(await page.$$eval(`css=span,div span,div div span`, els => els.length)).toBe(4); + expect(await page.$$eval(`css=#target,[attr="value\\ space"]`, els => els.length)).toBe(2); + expect(await page.$$eval(`css=#target,[data-testid="foo"],[attr="value\\ space"]`, els => els.length)).toBe(4); + expect(await page.$$eval(`css=#target,[data-testid="foo"],[attr="value\\ space"],span`, els => els.length)).toBe(4); + }); + + it('should keep dom order with comma separated list', async({page}) => { + await page.setContent(`
`); + expect(await page.$$eval(`css=span,div`, els => els.map(e => e.nodeName).join(','))).toBe('SPAN,DIV'); + expect(await page.$$eval(`css=div,span`, els => els.map(e => e.nodeName).join(','))).toBe('SPAN,DIV'); + expect(await page.$$eval(`css=span div, div`, els => els.map(e => e.nodeName).join(','))).toBe('DIV'); + expect(await page.$$eval(`*css=section >> css=div,span`, els => els.map(e => e.nodeName).join(','))).toBe('SECTION'); + expect(await page.$$eval(`css=section >> *css=div >> css=x,y`, els => els.map(e => e.nodeName).join(','))).toBe('DIV'); + expect(await page.$$eval(`css=section >> *css=div,span >> css=x,y`, els => els.map(e => e.nodeName).join(','))).toBe('SPAN,DIV'); + expect(await page.$$eval(`css=section >> *css=div,span >> css=y`, els => els.map(e => e.nodeName).join(','))).toBe('SPAN,DIV'); + }); + + it('should work with comma inside text', async({page}) => { + await page.setContent(`
`); + expect(await page.$eval(`css=div[attr="hello,world!"]`, e => e.outerHTML)).toBe('
'); + expect(await page.$eval(`css=[attr="hello,world!"]`, e => e.outerHTML)).toBe('
'); + expect(await page.$eval(`css=div[attr='hello,world!']`, e => e.outerHTML)).toBe('
'); + expect(await page.$eval(`css=[attr='hello,world!']`, e => e.outerHTML)).toBe('
'); + expect(await page.$eval(`css=div[attr="hello,world!"],span`, e => e.outerHTML)).toBe(''); + }); }); describe('attribute selector', () => { diff --git a/tsconfig.json b/tsconfig.json index 37a8b36a86..8b6528ed5a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "target": "ESNext", "module": "commonjs", - "lib": ["esnext", "dom"], + "lib": ["esnext", "dom", "DOM.Iterable"], "sourceMap": true, "rootDir": "./src", "outDir": "./lib",