fix(role): closed <details> are considered hidden (#20726)

Fixes #20610.

packages/playwright-core/src/server/injected/domUtils.ts

@@ -55,11 +55,38 @@ export function closestCrossShadow(element: Element | undefined, css: string): E
   }
 }
 
+export function getElementComputedStyle(element: Element, pseudo?: string): CSSStyleDeclaration | undefined {
+  return element.ownerDocument && element.ownerDocument.defaultView ? element.ownerDocument.defaultView.getComputedStyle(element, pseudo) : undefined;
+}
+
+export function isElementStyleVisibilityVisible(element: Element, style?: CSSStyleDeclaration): boolean {
+  style = style ?? getElementComputedStyle(element);
+  if (!style)
+    return true;
+  // Element.checkVisibility checks for content-visibility and also looks at
+  // styles up the flat tree including user-agent ShadowRoots, such as the
+  // details element for example.
+  // @ts-ignore Typescript doesn't know that checkVisibility exists yet.
+  if (Element.prototype.checkVisibility) {
+    // @ts-ignore Typescript doesn't know that checkVisibility exists yet.
+    if (!element.checkVisibility({ checkOpacity: false, checkVisibilityCSS: false }))
+      return false;
+  } else {
+    // Manual workaround for WebKit that does not have checkVisibility.
+    const detailsOrSummary = element.closest('details,summary');
+    if (detailsOrSummary !== element && detailsOrSummary?.nodeName === 'DETAILS' && !(detailsOrSummary as HTMLDetailsElement).open)
+      return false;
+  }
+  if (style.visibility !== 'visible')
+    return false;
+  return true;
+}
+
 export function isElementVisible(element: Element): boolean {
   // Note: this logic should be similar to waitForDisplayedAtStablePosition() to avoid surprises.
-  if (!element.ownerDocument || !element.ownerDocument.defaultView)
+  const style = getElementComputedStyle(element);
+  if (!style)
     return true;
-  const style = element.ownerDocument.defaultView.getComputedStyle(element);
   if (style.display === 'contents') {
     // display:contents is not rendered itself, but its child nodes are.
     for (let child = element.firstChild; child; child = child.nextSibling) {
@@ -70,13 +97,7 @@ export function isElementVisible(element: Element): boolean {
     }
     return false;
   }
-  // Element.checkVisibility checks for content-visibility and also looks at
+  if (!isElementStyleVisibilityVisible(element, style))
-  // styles up the flat tree including user-agent ShadowRoots, such as the
-  // details element for example.
-  // @ts-ignore Typescript doesn't know that checkVisibility exists yet.
-  if (Element.prototype.checkVisibility && !element.checkVisibility({ checkOpacity: false, checkVisibilityCSS: false }))
-    return false;
-  if (!style || style.visibility === 'hidden')
     return false;
   const rect = element.getBoundingClientRect();
   return rect.width > 0 && rect.height > 0;

packages/playwright-core/src/server/injected/roleUtils.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { closestCrossShadow, enclosingShadowRootOrDocument, parentElementOrShadowHost } from './domUtils';
+import { closestCrossShadow, enclosingShadowRootOrDocument, getElementComputedStyle, isElementStyleVisibilityVisible, parentElementOrShadowHost } from './domUtils';
 
 function hasExplicitAccessibleName(e: Element) {
   return e.hasAttribute('aria-label') || e.hasAttribute('aria-labelledby');
@@ -225,24 +225,23 @@ function getAriaBoolean(attr: string | null) {
   return attr === null ? undefined : attr.toLowerCase() === 'true';
 }
 
-function getComputedStyle(element: Element, pseudo?: string): CSSStyleDeclaration | undefined {
-  return element.ownerDocument && element.ownerDocument.defaultView ? element.ownerDocument.defaultView.getComputedStyle(element, pseudo) : undefined;
-}
-
 // https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion, but including "none" and "presentation" roles
 // https://www.w3.org/TR/wai-aria-1.2/#aria-hidden
 export function isElementHiddenForAria(element: Element, cache: Map<Element, boolean>): boolean {
   if (['STYLE', 'SCRIPT', 'NOSCRIPT', 'TEMPLATE'].includes(element.tagName))
     return true;
-  const style: CSSStyleDeclaration | undefined = getComputedStyle(element);
+  // Note: <option> inside <select> are not affected by visibility or content-visibility.
-  if (!style || style.visibility === 'hidden')
+  // Same goes for <slot>.
+  const isOptionInsideSelect = element.nodeName === 'OPTION' && !!element.closest('select');
+  const isSlot = element.nodeName === 'SLOT';
+  if (!isOptionInsideSelect && !isSlot && !isElementStyleVisibilityVisible(element))
     return true;
   return belongsToDisplayNoneOrAriaHidden(element, cache);
 }
 
 function belongsToDisplayNoneOrAriaHidden(element: Element, cache: Map<Element, boolean>): boolean {
   if (!cache.has(element)) {
-    const style = getComputedStyle(element);
+    const style = getElementComputedStyle(element);
     let hidden = !style || style.display === 'none' || getAriaBoolean(element.getAttribute('aria-hidden')) === true;
     if (!hidden) {
       const parent = parentElementOrShadowHost(element);
@@ -603,7 +602,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN
       if (skipSlotted && (node as Element | Text).assignedSlot)
         return;
       if (node.nodeType === 1 /* Node.ELEMENT_NODE */) {
-        const display = getComputedStyle(node as Element)?.getPropertyValue('display') || 'inline';
+        const display = getElementComputedStyle(node as Element)?.getPropertyValue('display') || 'inline';
         let token = getElementAccessibleNameInternal(node as Element, childOptions);
         // SPEC DIFFERENCE.
         // Spec says "append the result to the accumulated text", assuming "with space".
@@ -617,7 +616,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN
         tokens.push(node.textContent || '');
       }
     };
-    tokens.push(getPseudoContent(getComputedStyle(element, '::before')));
+    tokens.push(getPseudoContent(getElementComputedStyle(element, '::before')));
     const assignedNodes = element.nodeName === 'SLOT' ? (element as HTMLSlotElement).assignedNodes() : [];
     if (assignedNodes.length) {
       for (const child of assignedNodes)
@@ -632,7 +631,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN
       for (const owned of getIdRefs(element, element.getAttribute('aria-owns')))
         visit(owned, true);
     }
-    tokens.push(getPseudoContent(getComputedStyle(element, '::after')));
+    tokens.push(getPseudoContent(getElementComputedStyle(element, '::after')));
     const accessibleName = tokens.join('');
     if (accessibleName.trim())
       return accessibleName;

tests/page/selectors-role.spec.ts

@@ -282,6 +282,22 @@ test('should filter hidden, unless explicitly asked for', async ({ page }) => {
     <button style="display:none">Never</button>
     <div id=host1></div>
     <div id=host2 style="display:none"></div>
+
+    <input name="one">
+    <details>
+      <summary>Open form</summary>
+      <label>
+         Label
+         <input name="two">
+      </label>
+    </details>
+
+    <select>
+      <option style="visibility:hidden">One</option>
+      <option style="display:none">Two</option>
+      <option>Three</option>
+    </select>
+
     <script>
       function addButton(host, text) {
         const root = host.attachShadow({ mode: 'open' });
@@ -329,6 +345,13 @@ test('should filter hidden, unless explicitly asked for', async ({ page }) => {
     `<button style="visibility:visible">Still here</button>`,
     `<button>Shadow1</button>`,
   ]);
+  expect(await page.locator(`role=textbox`).evaluateAll(els => els.map(e => e.outerHTML))).toEqual([
+    `<input name="one">`,
+  ]);
+  expect(await page.locator(`role=option`).evaluateAll(els => els.map(e => e.outerHTML))).toEqual([
+    `<option style="visibility:hidden">One</option>`,
+    `<option>Three</option>`,
+  ]);
 });
 
 test('should support name', async ({ page }) => {