Skip to content

Commit

Permalink
AX: textUnderElement for hidden elements targeted by aria-labelledby …
Browse files Browse the repository at this point in the history
…should expose all subtree text, not just the direct target of aria-labelledby

https://bugs.webkit.org/show_bug.cgi?id=271918
rdar://problem/125634439

Reviewed by Chris Fleizach.

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

> The result of LabelledBy Recursion in combination with Hidden Not Referenced means that user agents MUST include all
> nodes in the subtree as part of the accessible name or accessible description, when the node referenced by aria-labelledby
> or aria-describedby is hidden.

Prior to this patch, we correctly exposed the text of the children directly underneath DOM hidden nodes targeted by
aria-labelledby, but not grandchildren or deeper, which is what the spec requires.

This allows us to pass 7 more WPT subtests.

This patch also renames AccessibilityTextUnderElementMode to TextUnderElementMode, as the former is very long and implicit
considering the type is defined in the Accessibility namespace.

* LayoutTests/imported/w3c/web-platform-tests/accname/name/comp_label-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/accname/name/comp_labelledby_hidden_nodes-expected.txt:
* LayoutTests/platform/gtk/imported/w3c/web-platform-tests/accname/name/comp_label-expected.txt:
* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::TextUnderElementMode::TextUnderElementMode):
(WebCore::AccessibilityTextUnderElementMode::AccessibilityTextUnderElementMode): Deleted.
* Source/WebCore/accessibility/AXLogger.cpp:
(WebCore::operator<<):
* Source/WebCore/accessibility/AccessibilityMathMLElement.cpp:
(WebCore::AccessibilityMathMLElement::textUnderElement const):
* Source/WebCore/accessibility/AccessibilityMathMLElement.h:
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::visibleText const):
(WebCore::shouldUseAccessibilityObjectInnerText):
(WebCore::AccessibilityNodeObject::textUnderElement const):
(WebCore::AccessibilityNodeObject::title const):
(WebCore::accessibleNameForNode):
* Source/WebCore/accessibility/AccessibilityNodeObject.h:
* Source/WebCore/accessibility/AccessibilityObject.h:
* Source/WebCore/accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::textUnderElement const):
(WebCore::AccessibilityRenderObject::shouldGetTextFromNode const):
* Source/WebCore/accessibility/AccessibilityRenderObject.h:
* Source/WebCore/accessibility/atspi/AccessibilityObjectTextAtspi.cpp:
(WebCore::AccessibilityObjectAtspi::text const):
(WebCore::AccessibilityObject::getLengthForTextRange const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::textUnderElement const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:

Canonical link: https://commits.webkit.org/276864@main
  • Loading branch information
twilco committed Mar 31, 2024
1 parent 1a5937c commit b4c7de1
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,7 @@ FAIL aria-label undefined on img w/o alt assert_equals: <img aria-label="undefin
FAIL aria-label undefined on img w/ empty alt assert_equals: <img alt="" aria-label="undefined" data-expectedlabel="" data-testname="aria-label undefined on img w/ empty alt" class="ex"> expected "" but got "undefined"
FAIL aria-label undefined on img w/o alt but w/ title assert_equals: <img aria-label="undefined" data-expectedlabel="title" data-testname="aria-label undefined on img w/o alt but w/ title" class="ex"> expected "title" but got "undefined"
FAIL aria-label undefined on img w/ empty alt but w/ title assert_equals: <img alt="" aria-label="undefined" data-expectedlabel="title" data-testname="aria-label undefined on img w/ empty alt but w/ title" class="ex"> expected "title" but got "undefined"
FAIL button's hidden referenced name (display:none) supercedes aria-label assert_equals: <button aria-labelledby="span1" aria-label="foo" data-expectedlabel="label" data-testname="button's hidden referenced name (display:none) supercedes aria-label" class="ex">
<span id="span1" style="display:none;">
<span id="span2" style="display:none;">label</span>
</span>
x
</button> expected "label" but got "foo"
PASS button's hidden referenced name (display:none) supercedes aria-label
PASS button's hidden referenced name (visibility:hidden) supercedes aria-label
FAIL button's hidden referenced name (visibility:hidden) with hidden aria-labelledby traversal falls back to aria-label assert_equals: <button aria-labelledby="span4" aria-label="foo" data-expectedlabel="foo" data-testname="button's hidden referenced name (visibility:hidden) with hidden aria-labelledby traversal falls back to aria-label" class="ex">
<span id="span4">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ PASS button with aria-labelledby using span without display:none (with nested di
PASS button with aria-labelledby using display:none hidden span (with nested sibling spans)
PASS button with aria-labelledby using span without display:none (with nested display:none sibling spans)
PASS button with aria-labelledby using span with display:none (with nested display:inline sibling spans)
FAIL button with aria-labelledby using visibility:hidden span (with nested span) assert_equals: <button aria-labelledby="b11" data-expectedlabel="foo bar" data-testname="button with aria-labelledby using visibility:hidden span (with nested span)" class="ex">x</button> expected "foo bar" but got "foo"
FAIL button with aria-labelledby using visibility:hidden span (with nested spans, depth 2) assert_equals: <button aria-labelledby="b21" data-expectedlabel="foo bar baz" data-testname="button with aria-labelledby using visibility:hidden span (with nested spans, depth 2)" class="ex">x</button> expected "foo bar baz" but got "foo"
PASS button with aria-labelledby using visibility:hidden span (with nested span)
PASS button with aria-labelledby using visibility:hidden span (with nested spans, depth 2)
PASS button with aria-labelledby using span without visibility:hidden (with nested visibility:hidden spans, depth 2)
FAIL button with aria-labelledby using visibility:hidden hidden span (with nested sibling spans) assert_equals: <button aria-labelledby="b41" data-expectedlabel="foo bar baz" data-testname="button with aria-labelledby using visibility:hidden hidden span (with nested sibling spans)" class="ex">x</button> expected "foo bar baz" but got "foo"
PASS button with aria-labelledby using visibility:hidden hidden span (with nested sibling spans)
PASS button with aria-labelledby using span without visibility:hidden (with nested visibility:hidden sibling spans)
PASS button with aria-labelledby using span with visibility:hidden (with nested visibility:visible sibling spans)
FAIL button with aria-labelledby using visibility:collapse span (with nested span) assert_equals: <button aria-labelledby="c11" data-expectedlabel="foo bar" data-testname="button with aria-labelledby using visibility:collapse span (with nested span)" class="ex">x</button> expected "foo bar" but got "foo"
FAIL button with aria-labelledby using visibility:collapse span (with nested spans, depth 2) assert_equals: <button aria-labelledby="c21" data-expectedlabel="foo bar baz" data-testname="button with aria-labelledby using visibility:collapse span (with nested spans, depth 2)" class="ex">x</button> expected "foo bar baz" but got "foo"
PASS button with aria-labelledby using visibility:collapse span (with nested span)
PASS button with aria-labelledby using visibility:collapse span (with nested spans, depth 2)
PASS button with aria-labelledby using span without visibility:collapse (with nested visibility:visible spans, depth 2)
FAIL button with aria-labelledby using visibility:collapse span (with nested sibling spans) assert_equals: <button aria-labelledby="c41" data-expectedlabel="foo bar baz" data-testname="button with aria-labelledby using visibility:collapse span (with nested sibling spans)" class="ex">x</button> expected "foo bar baz" but got "foo"
PASS button with aria-labelledby using visibility:collapse span (with nested sibling spans)
PASS button with aria-labelledby using span without visibility:collapse (with nested visibility:collapse sibling spans)
PASS button with aria-labelledby using span with visibility:collapse (with nested visible sibling spans)
PASS button with aria-labelledby using aria-hidden span (with nested span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,7 @@ FAIL aria-label undefined on img w/o alt assert_equals: <img aria-label="undefin
FAIL aria-label undefined on img w/ empty alt assert_equals: <img alt="" aria-label="undefined" data-expectedlabel="" data-testname="aria-label undefined on img w/ empty alt" class="ex"> expected "" but got "undefined"
FAIL aria-label undefined on img w/o alt but w/ title assert_equals: <img aria-label="undefined" data-expectedlabel="title" data-testname="aria-label undefined on img w/o alt but w/ title" class="ex"> expected "title" but got "undefined"
FAIL aria-label undefined on img w/ empty alt but w/ title assert_equals: <img alt="" aria-label="undefined" data-expectedlabel="title" data-testname="aria-label undefined on img w/ empty alt but w/ title" class="ex"> expected "title" but got "undefined"
FAIL button's hidden referenced name (display:none) supercedes aria-label assert_equals: <button aria-labelledby="span1" aria-label="foo" data-expectedlabel="label" data-testname="button's hidden referenced name (display:none) supercedes aria-label" class="ex">
<span id="span1" style="display:none;">
<span id="span2" style="display:none;">label</span>
</span>
x
</button> expected "label" but got "foo"
PASS button's hidden referenced name (display:none) supercedes aria-label
PASS button's hidden referenced name (visibility:hidden) supercedes aria-label
FAIL button's hidden referenced name (visibility:hidden) with hidden aria-labelledby traversal falls back to aria-label assert_equals: <button aria-labelledby="span4" aria-label="foo" data-expectedlabel="foo" data-testname="button's hidden referenced name (visibility:hidden) with hidden aria-labelledby traversal falls back to aria-label" class="ex">
<span id="span4">
Expand Down
24 changes: 13 additions & 11 deletions Source/WebCore/accessibility/AXCoreObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -727,21 +727,22 @@ enum class AccessibilityOrientation {
Undefined,
};

struct AccessibilityTextUnderElementMode {
enum ChildrenInclusion {
TextUnderElementModeSkipIgnoredChildren,
TextUnderElementModeIncludeAllChildren,
TextUnderElementModeIncludeNameFromContentsChildren, // This corresponds to ARIA concept: nameFrom
struct TextUnderElementMode {
enum class Children : uint8_t {
SkipIgnoredChildren,
IncludeAllChildren,
IncludeNameFromContentsChildren, // This corresponds to ARIA concept: nameFrom
};

ChildrenInclusion childrenInclusion;
Children childrenInclusion;
bool includeFocusableContent;
bool considerHiddenState { true };
Node* ignoredChildNode;

AccessibilityTextUnderElementMode(ChildrenInclusion c = TextUnderElementModeSkipIgnoredChildren, bool i = false, Node* ignored = nullptr)
: childrenInclusion(c)
, includeFocusableContent(i)
, ignoredChildNode(ignored)
TextUnderElementMode(Children childrenInclusion = Children::SkipIgnoredChildren, bool includeFocusable = false, Node* ignoredChild = nullptr)
: childrenInclusion(childrenInclusion)
, includeFocusableContent(includeFocusable)
, ignoredChildNode(ignoredChild)
{ }
};

Expand Down Expand Up @@ -1140,7 +1141,7 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo

// Methods for determining accessibility text.
virtual String stringValue() const = 0;
virtual String textUnderElement(AccessibilityTextUnderElementMode = AccessibilityTextUnderElementMode()) const = 0;
virtual String textUnderElement(TextUnderElementMode = TextUnderElementMode()) const = 0;
virtual String text() const = 0;
virtual unsigned textLength() const = 0;
#if PLATFORM(COCOA)
Expand Down Expand Up @@ -1725,5 +1726,6 @@ WTF::TextStream& operator<<(WTF::TextStream&, const AXCoreObject&);
WTF::TextStream& operator<<(WTF::TextStream&, AccessibilityText);
WTF::TextStream& operator<<(WTF::TextStream&, AccessibilityTextSource);
WTF::TextStream& operator<<(WTF::TextStream&, AXRelationType);
WTF::TextStream& operator<<(WTF::TextStream&, const TextUnderElementMode&);

} // namespace WebCore
27 changes: 27 additions & 0 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,33 @@ TextStream& operator<<(TextStream& stream, AXRelationType relationType)
return stream;
}

TextStream& operator<<(WTF::TextStream& stream, const TextUnderElementMode& mode)
{
String childrenInclusion;
switch (mode.childrenInclusion) {
case TextUnderElementMode::Children::SkipIgnoredChildren:
childrenInclusion = "SkipIgnoredChildren"_s;
break;
case TextUnderElementMode::Children::IncludeAllChildren:
childrenInclusion = "IncludeAllChildren"_s;
break;
case TextUnderElementMode::Children::IncludeNameFromContentsChildren:
childrenInclusion = "IncludeNameFromContentsChildren"_s;
break;
default:
ASSERT_NOT_REACHED();
break;
}

stream << childrenInclusion << ", includeFocusableContent: " << mode.includeFocusableContent;
// Only log the non-default value of false to avoid noise.
if (!mode.considerHiddenState)
stream << ", considerHiddenState: 0";
if (mode.ignoredChildNode)
stream << ", ignoredChildNode: " << mode.ignoredChildNode;
return stream;
}

TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notification)
{
switch (notification) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ AccessibilityRole AccessibilityMathMLElement::determineAccessibilityRole()
return AccessibilityRole::MathElement;
}

String AccessibilityMathMLElement::textUnderElement(AccessibilityTextUnderElementMode mode) const
String AccessibilityMathMLElement::textUnderElement(TextUnderElementMode mode) const
{
if (m_isAnonymousOperator) {
UChar operatorChar = downcast<RenderMathMLOperator>(*m_renderer).textContent();
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityMathMLElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class AccessibilityMathMLElement : public AccessibilityRenderObject {

private:
AccessibilityRole determineAccessibilityRole() final;
String textUnderElement(AccessibilityTextUnderElementMode = AccessibilityTextUnderElementMode()) const override;
String textUnderElement(TextUnderElementMode = TextUnderElementMode()) const override;
String stringValue() const override;
bool isIgnoredElementWithinMathTree() const final;

Expand Down
26 changes: 17 additions & 9 deletions Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,7 @@ void AccessibilityNodeObject::visibleText(Vector<AccessibilityText>& textOrder)
return;

if (dependsOnTextUnderElement()) {
AccessibilityTextUnderElementMode mode;
TextUnderElementMode mode;

// Headings often include links as direct children. Those links need to be included in text under element.
if (isHeading())
Expand Down Expand Up @@ -2191,10 +2191,10 @@ void AccessibilityNodeObject::setIsExpanded(bool expand)

// When building the textUnderElement for an object, determine whether or not
// we should include the inner text of this given descendant object or skip it.
static bool shouldUseAccessibilityObjectInnerText(AccessibilityObject* obj, AccessibilityTextUnderElementMode mode)
static bool shouldUseAccessibilityObjectInnerText(AccessibilityObject* obj, TextUnderElementMode mode)
{
// Do not use any heuristic if we are explicitly asking to include all the children.
if (mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeAllChildren)
if (mode.childrenInclusion == TextUnderElementMode::Children::IncludeAllChildren)
return true;

// Consider this hypothetical example:
Expand All @@ -2215,7 +2215,7 @@ static bool shouldUseAccessibilityObjectInnerText(AccessibilityObject* obj, Acce
// containers with lots of children.

// ARIA states that certain elements are not allowed to expose their children content for name calculation.
if (mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren
if (mode.childrenInclusion == TextUnderElementMode::Children::IncludeNameFromContentsChildren
&& !obj->accessibleNameDerivesFromContent())
return false;

Expand Down Expand Up @@ -2259,7 +2259,7 @@ static void appendNameToStringBuilder(StringBuilder& builder, String&& text)
builder.append(WTFMove(text));
}

String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMode mode) const
String AccessibilityNodeObject::textUnderElement(TextUnderElementMode mode) const
{
Node* node = this->node();
if (RefPtr text = dynamicDowncast<Text>(node))
Expand All @@ -2272,17 +2272,25 @@ String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMo
// The Accname specification states that if the current node is hidden, and not directly
// referenced by aria-labelledby or aria-describedby, and is not a host language text
// alternative, the empty string should be returned.
if (isDOMHidden() && !isAriaVisible && !is<HTMLLabelElement>(node) && (node && !ancestorsOfType<HTMLCanvasElement>(*node).first())) {
if (mode.considerHiddenState && isDOMHidden() && !isAriaVisible && !is<HTMLLabelElement>(node) && (node && !ancestorsOfType<HTMLCanvasElement>(*node).first())) {
if (labelForObjects().isEmpty() && descriptionForObjects().isEmpty())
return { };

// This object is a hidden label or description for another object, so ignore hidden states for our
// subtree text under element traversals too (https://w3c.github.io/accname/#comp_labelledby).
//
// "The result of LabelledBy Recursion in combination with Hidden Not Referenced means that user
// agents MUST include all nodes in the subtree as part of the accessible name or accessible
// description, when the node referenced by aria-labelledby or aria-describedby is hidden."
mode.considerHiddenState = false;
}

StringBuilder builder;
for (RefPtr child = firstChild(); child; child = child->nextSibling()) {
if (mode.ignoredChildNode && child->node() == mode.ignoredChildNode)
continue;

bool shouldDeriveNameFromAuthor = (mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren && !child->accessibleNameDerivesFromContent());
bool shouldDeriveNameFromAuthor = (mode.childrenInclusion == TextUnderElementMode::Children::IncludeNameFromContentsChildren && !child->accessibleNameDerivesFromContent());
if (shouldDeriveNameFromAuthor) {
appendNameToStringBuilder(builder, accessibleNameForNode(child->node()));
continue;
Expand Down Expand Up @@ -2372,7 +2380,7 @@ String AccessibilityNodeObject::title() const
if (isLink())
return textUnderElement();
if (isHeading())
return textUnderElement(AccessibilityTextUnderElementMode(AccessibilityTextUnderElementMode::TextUnderElementModeSkipIgnoredChildren, true));
return textUnderElement(TextUnderElementMode(TextUnderElementMode::Children::SkipIgnoredChildren, true));

return { };
}
Expand Down Expand Up @@ -2556,7 +2564,7 @@ static String accessibleNameForNode(Node* node, Node* labelledbyNode)
String text;
if (axObject) {
if (axObject->accessibleNameDerivesFromContent())
text = axObject->textUnderElement(AccessibilityTextUnderElementMode(AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren, true, labelledbyNode));
text = axObject->textUnderElement(TextUnderElementMode(TextUnderElementMode::Children::IncludeNameFromContentsChildren, true, labelledbyNode));
} else
text = (element ? element->innerText() : node->textContent()).simplifyWhiteSpace(isUnicodeWhitespace);

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityNodeObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class AccessibilityNodeObject : public AccessibilityObject {

URL url() const override;
unsigned hierarchicalLevel() const override;
String textUnderElement(AccessibilityTextUnderElementMode = AccessibilityTextUnderElementMode()) const override;
String textUnderElement(TextUnderElementMode = TextUnderElementMode()) const override;
String accessibilityDescriptionForChildren() const;
String description() const override;
String helpText() const override;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
bool isARIAStaticText() const { return ariaRoleAttribute() == AccessibilityRole::StaticText; }
String stringValue() const override { return { }; }
bool dependsOnTextUnderElement() const;
String textUnderElement(AccessibilityTextUnderElementMode = AccessibilityTextUnderElementMode()) const override { return { }; }
String textUnderElement(TextUnderElementMode = TextUnderElementMode()) const override { return { }; }
String text() const override { return { }; }
unsigned textLength() const final;
#if ENABLE(AX_THREAD_TEXT_APIS)
Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ String AccessibilityRenderObject::helpText() const
return { };
}

String AccessibilityRenderObject::textUnderElement(AccessibilityTextUnderElementMode mode) const
String AccessibilityRenderObject::textUnderElement(TextUnderElementMode mode) const
{
if (!m_renderer)
return AccessibilityNodeObject::textUnderElement(mode);
Expand All @@ -680,7 +680,7 @@ String AccessibilityRenderObject::textUnderElement(AccessibilityTextUnderElement

// We use a text iterator for text objects AND for those cases where we are
// explicitly asking for the full text under a given element.
if (is<RenderText>(*m_renderer) || mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeAllChildren) {
if (is<RenderText>(*m_renderer) || mode.childrenInclusion == TextUnderElementMode::Children::IncludeAllChildren) {
// If possible, use a text iterator to get the text, so that whitespace
// is handled consistently.
Document* nodeDocument = nullptr;
Expand Down Expand Up @@ -728,7 +728,7 @@ String AccessibilityRenderObject::textUnderElement(AccessibilityTextUnderElement
return AccessibilityNodeObject::textUnderElement(mode);
}

bool AccessibilityRenderObject::shouldGetTextFromNode(AccessibilityTextUnderElementMode mode) const
bool AccessibilityRenderObject::shouldGetTextFromNode(TextUnderElementMode mode) const
{
if (!m_renderer)
return true;
Expand All @@ -737,7 +737,7 @@ bool AccessibilityRenderObject::shouldGetTextFromNode(AccessibilityTextUnderElem
// the child nodes to define positions. CSS tables and their anonymous descendants lack
// children with nodes.
if (m_renderer->isAnonymous() && m_renderer->isTablePart())
return mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeAllChildren;
return mode.childrenInclusion == TextUnderElementMode::Children::IncludeAllChildren;

// AccessibilityRenderObject::textUnderElement() calls rangeOfContents() to create the text
// range. rangeOfContents() does not include CSS-generated content.
Expand Down
Loading

0 comments on commit b4c7de1

Please sign in to comment.