fix(css selector): support comma-separated selector lists (#2120)

src/injected/cssSelectorEngine.ts

@@ -87,11 +87,13 @@ export function createCSSEngine(shadow: boolean): SelectorEngine {
       //   return simple;
       // if (!shadow)
       //   return;
-      const parts = split(selector);
+      const selectors = split(selector);
-      if (!parts.length)
+      // 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, selectors, shadow);
-      return queryShadowInternal(root, root, parts, 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);
+      const selectors = split(selector);
-      if (parts.length) {
+      // Note: we do not just merge results produced by each selector, as that
-        parts.reverse();
+      // will not return them in the tree traversal order, but rather in the selectors
-        queryShadowAllInternal(root, root, parts, shadow, result);
+      // 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 {
+function queryShadowInternal(boundary: SelectorRoot, root: SelectorRoot, selectors: string[][], shadow: boolean): Element | undefined {
-  const matching = root.querySelectorAll(parts[0]);
+  let elements: NodeListOf<Element> | undefined;
-  for (let i = 0; i < matching.length; i++) {
+  if (selectors.length === 1) {
-    const element = matching[i];
+    // Fast path for a single selector - query only matching elements, not all.
-    if (parts.length === 1 || matches(element, parts, boundary))
+    const parts = selectors[0];
-      return element;
+    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('*');
+  if (!elements)
-  for (let i = 0; i < elements.length; i++) {
+    elements = root.querySelectorAll('*');
-    const element = elements[i];
+  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[]) {
+function queryShadowAllInternal(boundary: SelectorRoot, root: SelectorRoot, selectors: string[][], shadow: boolean, result: Element[]) {
-  const matching = root.querySelectorAll(parts[0]);
+  let elements: NodeListOf<Element> | undefined;
-  for (let i = 0; i < matching.length; i++) {
+  if (selectors.length === 1) {
-    const element = matching[i];
+    // Fast path for a single selector - query only matching elements, not all.
-    if (parts.length === 1 || matches(element, parts, boundary))
+    const parts = selectors[0];
-      result.push(element);
+    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);
+  // Visit shadow dom after the light dom to preserve the tree-traversal order.
-  const elements = root.querySelectorAll('*');
+  if (!shadow)
-  for (let i = 0; i < elements.length; i++) {
+    return;
-    const element = elements[i];
+  if ((root as Element).shadowRoot)
-    if (shadow && element.shadowRoot)
+    queryShadowAllInternal(boundary, (root as Element).shadowRoot!, selectors, shadow, result);
-      queryShadowAllInternal(boundary, element.shadowRoot, parts, 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 result: string[][] = [];
-  const append = () => {
+  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();
+  appendToResult();
-  return result;
+  return result.filter(parts => !!parts.length).map(parts => parts.reverse());
 }
 
 function test(engine: SelectorEngine) {

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');

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(`<section><span><div><x></x><y></y></div></span></section>`);
+    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(`<span></span><div attr="hello,world!"></div>`);
+    expect(await page.$eval(`css=div[attr="hello,world!"]`, e => e.outerHTML)).toBe('<div attr="hello,world!"></div>');
+    expect(await page.$eval(`css=[attr="hello,world!"]`, e => e.outerHTML)).toBe('<div attr="hello,world!"></div>');
+    expect(await page.$eval(`css=div[attr='hello,world!']`, e => e.outerHTML)).toBe('<div attr="hello,world!"></div>');
+    expect(await page.$eval(`css=[attr='hello,world!']`, e => e.outerHTML)).toBe('<div attr="hello,world!"></div>');
+    expect(await page.$eval(`css=div[attr="hello,world!"],span`, e => e.outerHTML)).toBe('<span></span>');
+  });
 });
 
 describe('attribute selector', () => {

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",