| 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: | P2 | Keywords: | GoodFirstBug, InRadar | ||||||||
| Version: | Other | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 470792 [details]
Patch
Created attachment 470793 [details]
Patch
(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()) (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(); } Created attachment 470843 [details]
Patch
> + 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. Found 1 new test failure: inspector/cpu-profiler/tracking.html 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]. |
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.