AX: Whitespace-only aria-labels should not be respected in accname calculation

• Component: Accessibility
• Status: RESOLVED FIXED

[Tyler Wilcock]

https://w3c.github.io/accname/#comp_label

> AriaLabel: Otherwise, if the current node has an aria-label attribute whose value is not undefined, not the empty string, nor, when trimmed of whitespace, is not the empty string:
>  1. If traversal of the current node is due to recursion and the current node is an embedded control, ignore aria-label and skip to rule Embedded Control.
>  2. Otherwise, return the value of aria-label.

[Radar WebKit Bug Importer]

<rdar://problem/126643869>

[Tyler Wilcock]

Created attachment 470970 [details]
Patch

[chris fleizach]

Comment on attachment 470970 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=470970&action=review

> Source/WebCore/accessibility/AccessibilityObject.h:544
> +    bool hasValidAriaLabel() const { return !getAttributeTrimmed(HTMLNames::aria_labelAttr).isEmpty(); }

Feel like we usually say "hasValidARIALabel". 
are we doing Aria in other places? I'm not a fan of that because ARIA is an acronym, not a name

> Source/WebCore/accessibility/AccessibilitySlider.cpp:-110
> -    if (auto* input = inputElement())

we handle this inputElement() thing in the single get attribute method?

[Tyler Wilcock]

Created attachment 470986 [details]
Patch

[Tyler Wilcock]

(In reply to chris fleizach from comment #3)
> Comment on attachment 470970 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=470970&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.h:544
> > +    bool hasValidAriaLabel() const { return !getAttributeTrimmed(HTMLNames::aria_labelAttr).isEmpty(); }
> 
> Feel like we usually say "hasValidARIALabel". 
> are we doing Aria in other places? I'm not a fan of that because ARIA is an
> acronym, not a name
We do use "Aria" in other places, e.g. determineAriaRoleAttribute, isPresentationalChildOfAriaRole, handleAriaExpandedChange, isNodeAriaVisible, remapAriaRoleDueToParent, and more.

But I think your point makes sense. I've switched it to hasValidARIALabel (but happy to change it back, I don't feel strongly either way).

> > Source/WebCore/accessibility/AccessibilitySlider.cpp:-110
> > -    if (auto* input = inputElement())
> 
> we handle this inputElement() thing in the single get attribute method?
Yeah, AccessibilitySlider was the only one who overrode getAttribute, and it did so needlessly. The default AccessibilityObject implementation has the same behavior — cast the underlying m_node to an element and get the attribute off of it. This function is called a lot, so de-virtualizing it is nice.

[Tyler Wilcock]

Created attachment 470994 [details]
Patch

[Tyler Wilcock]

Created attachment 471019 [details]
Patch

[EWS]

Committed 277781@main (04d118a7c6f4): <https://commits.webkit.org/277781@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 471019 [details].