RESOLVED FIXED256186
AX: Setting aria-hidden on a slot does not hide the slots assigned nodes 
https://bugs.webkit.org/show_bug.cgi?id=256186
Summary AX: Setting aria-hidden on a slot does not hide the slots assigned nodes
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
DetailsFormatted DiffDiff
Patch (14.03 KB, patch)
2024-04-06 15:34 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (13.91 KB, patch)
2024-04-09 19:00 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
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.