Revert "chore(role): cache element list by role (#29130)"

This reverts commit 1ce3ca2. Added a regression test.
microsoft · Mar 1, 2024 · 9994b6c · 9994b6c
1 parent 0f30cda
commit 9994b6c
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 59 deletions.
diff --git a/packages/playwright-core/src/server/injected/roleSelectorEngine.ts b/packages/playwright-core/src/server/injected/roleSelectorEngine.ts
@@ -16,10 +16,9 @@
 
 import type { SelectorEngine, SelectorRoot } from './selectorEngine';
 import { matchesAttributePart } from './selectorUtils';
-import { beginAriaCaches, endAriaCaches, getAriaChecked, getAriaDisabled, getAriaExpanded, getAriaLevel, getAriaPressed, getAriaSelected, getElementAccessibleName, getElementsByRole, isElementHiddenForAria, kAriaCheckedRoles, kAriaExpandedRoles, kAriaLevelRoles, kAriaPressedRoles, kAriaSelectedRoles } from './roleUtils';
+import { beginAriaCaches, endAriaCaches, getAriaChecked, getAriaDisabled, getAriaExpanded, getAriaLevel, getAriaPressed, getAriaRole, getAriaSelected, getElementAccessibleName, isElementHiddenForAria, kAriaCheckedRoles, kAriaExpandedRoles, kAriaLevelRoles, kAriaPressedRoles, kAriaSelectedRoles } from './roleUtils';
 import { parseAttributeSelector, type AttributeSelectorPart, type AttributeSelectorOperator } from '../../utils/isomorphic/selectorParser';
 import { normalizeWhiteSpace } from '../../utils/isomorphic/stringUtils';
-import { isInsideScope } from './domUtils';
 
 type RoleEngineOptions = {
   role: string;
@@ -126,27 +125,26 @@ function validateAttributes(attrs: AttributeSelectorPart[], role: string): RoleE
 }
 
 function queryRole(scope: SelectorRoot, options: RoleEngineOptions, internal: boolean): Element[] {
-  const doc = scope.nodeType === 9 /* Node.DOCUMENT_NODE */ ? scope as Document : scope.ownerDocument;
-  const elements = doc ? getElementsByRole(doc, options.role) : [];
-  return elements.filter(element => {
-    if (!isInsideScope(scope, element))
-      return false;
+  const result: Element[] = [];
+  const match = (element: Element) => {
+    if (getAriaRole(element) !== options.role)
+      return;
     if (options.selected !== undefined && getAriaSelected(element) !== options.selected)
-      return false;
+      return;
     if (options.checked !== undefined && getAriaChecked(element) !== options.checked)
-      return false;
+      return;
     if (options.pressed !== undefined && getAriaPressed(element) !== options.pressed)
-      return false;
+      return;
     if (options.expanded !== undefined && getAriaExpanded(element) !== options.expanded)
-      return false;
+      return;
     if (options.level !== undefined && getAriaLevel(element) !== options.level)
-      return false;
+      return;
     if (options.disabled !== undefined && getAriaDisabled(element) !== options.disabled)
-      return false;
+      return;
     if (!options.includeHidden) {
       const isHidden = isElementHiddenForAria(element);
       if (isHidden)
-        return false;
+        return;
     }
     if (options.name !== undefined) {
       // Always normalize whitespace in the accessible name.
@@ -157,10 +155,25 @@ function queryRole(scope: SelectorRoot, options: RoleEngineOptions, internal: bo
       if (internal && !options.exact && options.nameOp === '=')
         options.nameOp = '*=';
       if (!matchesAttributePart(accessibleName, { name: '', jsonPath: [], op: options.nameOp || '=', value: options.name, caseSensitive: !!options.exact }))
-        return false;
+        return;
     }
-    return true;
-  });
+    result.push(element);
+  };
+
+  const query = (root: Element | ShadowRoot | Document) => {
+    const shadows: ShadowRoot[] = [];
+    if ((root as Element).shadowRoot)
+      shadows.push((root as Element).shadowRoot!);
+    for (const element of root.querySelectorAll('*')) {
+      match(element);
+      if (element.shadowRoot)
+        shadows.push(element.shadowRoot);
+    }
+    shadows.forEach(query);
+  };
+
+  query(scope);
+  return result;
 }
 
 export function createRoleEngine(internal: boolean): SelectorEngine {

diff --git a/packages/playwright-core/src/server/injected/roleUtils.ts b/packages/playwright-core/src/server/injected/roleUtils.ts
@@ -845,51 +845,11 @@ function getAccessibleNameFromAssociatedLabels(labels: Iterable<HTMLLabelElement
   })).filter(accessibleName => !!accessibleName).join(' ');
 }
 
-export function getElementsByRole(document: Document, role: string): Element[] {
-  if (document === cacheElementsByRoleDocument)
-    return cacheElementsByRole!.get(role) || [];
-  const map = calculateElementsByRoleMap(document);
-  if (cachesCounter) {
-    cacheElementsByRoleDocument = document;
-    cacheElementsByRole = map;
-  }
-  return map.get(role) || [];
-}
-
-function calculateElementsByRoleMap(document: Document) {
-  const result = new Map<string, Element[]>();
-
-  const visit = (root: Element | ShadowRoot | Document) => {
-    const shadows: ShadowRoot[] = [];
-    if ((root as Element).shadowRoot)
-      shadows.push((root as Element).shadowRoot!);
-    for (const element of root.querySelectorAll('*')) {
-      const role = getAriaRole(element);
-      if (role) {
-        let list = result.get(role);
-        if (!list) {
-          list = [];
-          result.set(role, list);
-        }
-        list.push(element);
-      }
-      if (element.shadowRoot)
-        shadows.push(element.shadowRoot);
-    }
-    shadows.forEach(visit);
-  };
-  visit(document);
-
-  return result;
-}
-
 let cacheAccessibleName: Map<Element, string> | undefined;
 let cacheAccessibleNameHidden: Map<Element, string> | undefined;
 let cacheIsHidden: Map<Element, boolean> | undefined;
 let cachePseudoContentBefore: Map<Element, string> | undefined;
 let cachePseudoContentAfter: Map<Element, string> | undefined;
-let cacheElementsByRole: Map<string, Element[]> | undefined;
-let cacheElementsByRoleDocument: Document | undefined;
 let cachesCounter = 0;
 
 export function beginAriaCaches() {
@@ -908,7 +868,5 @@ export function endAriaCaches() {
     cacheIsHidden = undefined;
     cachePseudoContentBefore = undefined;
     cachePseudoContentAfter = undefined;
-    cacheElementsByRole = undefined;
-    cacheElementsByRoleDocument = undefined;
   }
 }
diff --git a/tests/page/selectors-role.spec.ts b/tests/page/selectors-role.spec.ts
@@ -484,3 +484,20 @@ test('should support output accessible name', async ({ page }) => {
   await page.setContent(`<label>Output1<output>output</output></label>`);
   await expect(page.getByRole('status', { name: 'Output1' })).toBeVisible();
 });
+
+test('should not match scope by default', async ({ page }) => {
+  await page.setContent(`
+    <ul>
+      <li aria-label="Parent list">
+        Parent list
+        <ul>
+          <li>child 1</li>
+          <li>child 2</li>
+        </ul>
+      </li>
+    </ul>
+  `);
+  const children = page.getByRole('listitem', { name: 'Parent list' }).getByRole('listitem');
+  await expect(children).toHaveCount(2);
+  await expect(children).toHaveText(['child 1', 'child 2']);
+});