From 272fad0e5e82b0d49fcc39028e9eec74c5f91ed8 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 1 Mar 2024 08:42:14 -0800 Subject: [PATCH] Revert "chore(role): cache element list by role (#29130)" This reverts commit 1ce3ca25a2676ca773a736035699cf3fe0c35d0d. Added a regression test. --- .../src/server/injected/roleSelectorEngine.ts | 47 ++++++++++++------- .../src/server/injected/roleUtils.ts | 42 ----------------- tests/page/selectors-role.spec.ts | 17 +++++++ 3 files changed, 47 insertions(+), 59 deletions(-) diff --git a/packages/playwright-core/src/server/injected/roleSelectorEngine.ts b/packages/playwright-core/src/server/injected/roleSelectorEngine.ts index 56643199e52fb5..b647f81b8eb45b 100644 --- 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 index a42b60233e5d2c..6cecb1d5a57d80 100644 --- 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 !!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(); - - 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 | undefined; let cacheAccessibleNameHidden: Map | undefined; let cacheIsHidden: Map | undefined; let cachePseudoContentBefore: Map | undefined; let cachePseudoContentAfter: Map | undefined; -let cacheElementsByRole: Map | 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 index d936f1b2291f85..0d7d1f3fcb165d 100644 --- 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(``); await expect(page.getByRole('status', { name: 'Output1' })).toBeVisible(); }); + +test('should not match scope by default', async ({ page }) => { + await page.setContent(` +
    +
  • + Parent list +
      +
    • child 1
      • +
    • child 2
      • +
    +
    • +
+ `); + const children = page.getByRole('listitem', { name: 'Parent list' }).getByRole('listitem'); + await expect(children).toHaveCount(2); + await expect(children).toHaveText(['child 1', 'child 2']); +});