Bug 256186

Summary:

AX: Setting aria-hidden on a slot does not hide the slots assigned nodes

Product:

WebKit

Reporter:

Tyler Wilcock <tyler_w>

Component:

Accessibility

Assignee:

Tyler Wilcock <tyler_w>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aboxhall, andresg_22, annulen, apinheiro, cdumez, cfleizach, cmarcelo, dmazzoni, esprehn+autocc, ews-watchlist, gyuyoung.kim, jcraig, kangil.han, ryuan.choi, samuel_white, sergio, webkit-bug-importer

Priority:

Keywords:

GoodFirstBug, InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Tyler Wilcock

Reported 2023-05-01 16:49:16 PDT

Testcase: <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> <html> <body> <x-button>slot<span>with child</span></x-button> <script> class XButton extends HTMLElement { constructor() { super(); this.attachShadow({ mode: "open", delegatesFocus: true }); const fragment = document.createRange().createContextualFragment("<input id='button' aria-labelledby='slot' type=button><slot id='slot' aria-hidden='true'></slot>"); this.shadowRoot.append(fragment.cloneNode(true)); } } customElements.define("x-button", XButton); </script> </body> </html> The only element exposed on this page should be the button, but WebKit exposes the text "slot with child" as well. Firefox and Chrome do not behave this way.

Attachments
Patch (13.98 KB, patch) 2024-04-06 13:24 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (14.03 KB, patch) 2024-04-06 15:34 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (13.91 KB, patch) 2024-04-09 19:00 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2023-05-01 16:49:34 PDT

<rdar://problem/108762653>

Tyler Wilcock

Comment 2 2024-04-06 13:24:41 PDT

Created attachment 470792 [details] Patch

Tyler Wilcock

Comment 3 2024-04-06 15:34:38 PDT

Created attachment 470793 [details] Patch

Andres Gonzalez

Comment 4 2024-04-09 18:39:31 PDT

(In reply to Tyler Wilcock from comment #3) > Created attachment 470793 [details] > Patch diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp index fac5fc7237b3..27df78d61832 100644 --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -310,15 +310,10 @@ AXObjectCache::~AXObjectCache() bool AXObjectCache::isModalElement(Element& element) const { - bool hasDialogRole = nodeHasRole(&element, "dialog"_s) || nodeHasRole(&element, "alertdialog"_s); - AtomString modalValue = element.attributeWithoutSynchronization(aria_modalAttr); - if (modalValue.isNull()) { - if (auto* defaultARIA = element.customElementDefaultARIAIfExists()) - modalValue = defaultARIA->valueForAttribute(element, aria_modalAttr); + if (nodeHasRole(&element, "dialog"_s) || nodeHasRole(&element, "alertdialog"_s)) { + if (equalLettersIgnoringASCIICase(element.attributeIncludingDefaultARIA(aria_modalAttr), "true"_s)) + return true; } AG: it may be more readable as if ((blah || blah) && blah) return true; Saving one if statement and one indentation. - bool isAriaModal = equalLettersIgnoringASCIICase(modalValue, "true"_s); - if (hasDialogRole && isAriaModal) - return true; RefPtr dialog = dynamicDowncast<HTMLDialogElement>(element); return dialog && dialog->isModal(); @@ -585,11 +580,7 @@ bool nodeHasRole(Node* node, StringView role) if (!element) return false; - auto roleValue = element->attributeWithoutSynchronization(roleAttr); - if (roleValue.isNull()) { - if (auto* defaultARIA = element->customElementDefaultARIAIfExists()) - roleValue = defaultARIA->valueForAttribute(*element, roleAttr); - } + auto roleValue = element->attributeIncludingDefaultARIA(roleAttr); if (role.isNull()) return roleValue.isEmpty(); if (roleValue.isEmpty())

Andres Gonzalez

Comment 5 2024-04-09 18:52:30 PDT

(In reply to Tyler Wilcock from comment #3) > Created attachment 470793 [details] > Patch diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h index 04fcaebdcebf..25d590032e6e 100644 --- a/Source/WebCore/dom/Element.h +++ b/Source/WebCore/dom/Element.h @@ -166,6 +166,7 @@ public: // attribute or one of the SVG animatable attributes. inline bool hasAttributeWithoutSynchronization(const QualifiedName&) const; inline const AtomString& attributeWithoutSynchronization(const QualifiedName&) const; + inline const AtomString& attributeIncludingDefaultARIA(const QualifiedName&) const; AG: maybe attributeWithDefaultARIA or attributeUsingDefaultARIA } #if DUMP_NODE_STATISTICS bool hasNamedNodeMap() const; @@ -381,7 +382,7 @@ public: CustomElementDefaultARIA& customElementDefaultARIA(); CheckedRef<CustomElementDefaultARIA> checkedCustomElementDefaultARIA(); - CustomElementDefaultARIA* customElementDefaultARIAIfExists(); + CustomElementDefaultARIA* customElementDefaultARIAIfExists() const; bool isInActiveChain() const { return isUserActionElement() && isUserActionElementInActiveChain(); } bool active() const { return isUserActionElement() && isUserActionElementActive(); }

Tyler Wilcock

Comment 6 2024-04-09 19:00:48 PDT

Created attachment 470843 [details] Patch

Tyler Wilcock

Comment 7 2024-04-09 19:02:28 PDT

> + inline const AtomString& attributeIncludingDefaultARIA(const > QualifiedName&) const; > > AG: maybe attributeWithDefaultARIA or attributeUsingDefaultARIA } TW: Went with attributeWithDefaultARIA in the latest revision. > AG: it may be more readable as > if ((blah || blah) && blah) > return true; > Saving one if statement and one indentation. TW: Changed! Thanks for the review.

EWS

Comment 8 2024-04-10 13:02:30 PDT

Found 1 new test failure: inspector/cpu-profiler/tracking.html

EWS

Comment 9 2024-04-10 22:38:21 PDT

Committed 277359@main (8ee95fe74bfa): <https://commits.webkit.org/277359@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 470843 [details].

Note You need to log in before you can comment on or make changes to this bug.