Skip to content

Commit

Permalink
Cherry-pick 6d76fb1. rdar://115226509
Browse files Browse the repository at this point in the history
    AX: No accessibility label is exposed for display:contents role="treeitem" elements that have a newline before their child text
    https://bugs.webkit.org/show_bug.cgi?id=261376
    rdar://problem/115226509

    Reviewed by Chris Fleizach.

    This patch fixes two bugs:

      1. No accessibility label is exposed for display:contents role="treeitem" elements that have a newline before their child text

      2. Some display:contents role="treeitem" expose text for other
         elements, resulting in that text being doubly-exposed

    The first was caused by a lack of resilience in AccessibilityNodeObject::firstChild.
    AccessibilityNodeObject::textUnderElement relies on iterating through its accessible children like so:

    for (RefPtr child = firstChild(); child; child = child->nextSibling()) {
       ... compute AX text ...
    }

    However, for elements that AXObjectCache::getOrCreate fails to create an
    object for, we would never iterate because firstChild returned nullptr,
    thus resulting in no AX text being gathered. AccessibilityNodeObject::firstChild
    now detects this, and iterates until it finds the first accessible child.

    The second bug was caused by AccessibilityNodeObject::textUnderElement
    recursively calling and textUnderElement for an element that was not
    it's child. With this patch, we detect this and continue the loop when
    it happens.

    New testcases added to accessibility/display-contents/tree-and-treeitems.html.

    * LayoutTests/accessibility/display-contents/tree-and-treeitems-expected.txt:
    * LayoutTests/accessibility/display-contents/tree-and-treeitems.html:
    * Source/WebCore/accessibility/AXLogger.cpp:
    (WebCore::operator<<):
    * Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
    (WebCore::AccessibilityNodeObject::firstChild const):
    (WebCore::AccessibilityNodeObject::textUnderElement const):
    * Source/WebCore/accessibility/AccessibilityObjectInterface.h:

    Canonical link: https://commits.webkit.org/267839@main
Identifier: 265423.1036@safari-7616-branch
  • Loading branch information
twilco authored and MyahCobbs committed Sep 25, 2023
1 parent 3f96a86 commit 0558fcb
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,31 @@ PASS: treeitem1.isExpanded === true
PASS: treeitem1.hierarchicalLevel === 0
PASS: treeitem1.isAttributeSupported('AXValue') === true
PASS: canSetDisclosing === true
treeitem1 text:
AXTitle:
AXDescription: Animals
AXHelp:

PASS: treeitem2.role === 'AXRole: AXRow'
PASS: treeitem2.subrole === 'AXSubrole: AXOutlineRow'
PASS: treeitem2.isExpanded === false
PASS: treeitem2.hierarchicalLevel === 1
PASS: treeitem2.disclosedByRow().isEqual(treeitem1) === true
PASS: canSetDisclosing === false
treeitem2 text:
AXTitle:
AXDescription: Birds
AXHelp:

PASS: treeitem3.stringValue === 'AXValue: Birds'
PASS: canSetDisclosing === false
PASS: treeitem3.role === 'AXRole: AXRow'
PASS: treeitem3.subrole === 'AXSubrole: AXOutlineRow'
PASS: treeitem3.isExpanded === true
treeitem3 text:
AXTitle:
AXDescription: Cats
AXHelp:
PASS: treeitem3.isSelected === false
PASS: treeitem3.isSelected === true
PASS: selectedRow.isEqual(treeitem3) === true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
output += expect("treeitem1.isAttributeSupported('AXValue')", "true");
var canSetDisclosing = treeitem1.isAttributeSettable('AXDisclosing');
output += expect("canSetDisclosing", "true");
output += `treeitem1 text:\n${platformTextAlternatives(treeitem1)}\n\n`;

var treeitem2 = treeitem1.disclosedRowAtIndex(0);
output += expect("treeitem2.role", "'AXRole: AXRow'");
Expand All @@ -72,6 +73,7 @@
output += expect("treeitem2.disclosedByRow().isEqual(treeitem1)", "true");
canSetDisclosing = treeitem2.isAttributeSettable('AXDisclosing');
output += expect("canSetDisclosing", "false");
output += `treeitem2 text:\n${platformTextAlternatives(treeitem2)}\n\n`;

var treeitem3 = treeitem2.childAtIndex(0);
output += expect("treeitem3.stringValue", "'AXValue: Birds'");
Expand All @@ -83,6 +85,7 @@
output += expect("treeitem3.role", "'AXRole: AXRow'");
output += expect("treeitem3.subrole", "'AXSubrole: AXOutlineRow'");
output += expect("treeitem3.isExpanded", "true");
output += `treeitem3 text:\n${platformTextAlternatives(treeitem3)}\n`;

var selectedRow, treeitem4, treeitem5, treeitem6;
// Test that the row can be selected correctly.
Expand Down
40 changes: 40 additions & 0 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,46 @@ TextStream& operator<<(TextStream& stream, const AccessibilitySearchCriteria& cr
return stream;
}

TextStream& operator<<(TextStream& stream, AccessibilityTextSource source)
{
switch (source) {
case AccessibilityTextSource::Alternative:
stream << "Alternative";
break;
case AccessibilityTextSource::Children:
stream << "Children";
break;
case AccessibilityTextSource::Summary:
stream << "Summary";
break;
case AccessibilityTextSource::Help:
stream << "Help";
break;
case AccessibilityTextSource::Visible:
stream << "Visible";
break;
case AccessibilityTextSource::TitleTag:
stream << "TitleTag";
break;
case AccessibilityTextSource::Placeholder:
stream << "Placeholder";
break;
case AccessibilityTextSource::LabelByElement:
stream << "LabelByElement";
break;
case AccessibilityTextSource::Title:
stream << "Title";
break;
case AccessibilityTextSource::Subtitle:
stream << "Subtitle";
break;
case AccessibilityTextSource::Action:
stream << "Action";
break;
}
return stream;
}

TextStream& operator<<(TextStream& stream, AccessibilityObjectInclusion inclusion)
{
switch (inclusion) {
Expand Down
27 changes: 19 additions & 8 deletions Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,20 @@ void AccessibilityNodeObject::updateRole()

AccessibilityObject* AccessibilityNodeObject::firstChild() const
{
if (!node())
auto* currentChild = node() ? node()->firstChild() : nullptr;
if (!currentChild)
return nullptr;

Node* firstChild = node()->firstChild();

if (!firstChild)
auto* cache = axObjectCache();
if (!cache)
return nullptr;

auto objectCache = axObjectCache();
return objectCache ? objectCache->getOrCreate(firstChild) : nullptr;
auto* axCurrentChild = cache->getOrCreate(currentChild);
while (!axCurrentChild && currentChild) {
currentChild = currentChild->nextSibling();
axCurrentChild = cache->getOrCreate(currentChild);
}
return axCurrentChild;
}

AccessibilityObject* AccessibilityNodeObject::lastChild() const
Expand Down Expand Up @@ -2382,10 +2386,17 @@ String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMo
continue;
}
}


if (node) {
auto* childParentElement = child->node() ? child->node()->parentElement() : nullptr;
// Do not take the textUnderElement for a different element (determined by child's element parent not being us). Otherwise we may doubly-expose the same text.
if (childParentElement && childParentElement != node && childParentElement->shadowHost() != node)
continue;
}

String childText = child->textUnderElement(mode);
if (childText.length())
appendNameToStringBuilder(builder, childText);
appendNameToStringBuilder(builder, WTFMove(childText));
}

return builder.toString().trim(deprecatedIsSpaceOrNewline).simplifyWhiteSpace(isHTMLSpaceButNotLineBreak);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2019,5 +2019,6 @@ WTF::TextStream& operator<<(WTF::TextStream&, AccessibilitySearchKey);
WTF::TextStream& operator<<(WTF::TextStream&, const AccessibilitySearchCriteria&);
WTF::TextStream& operator<<(WTF::TextStream&, AccessibilityObjectInclusion);
WTF::TextStream& operator<<(WTF::TextStream&, const AXCoreObject&);
WTF::TextStream& operator<<(WTF::TextStream&, AccessibilityTextSource);

} // namespace WebCore

0 comments on commit 0558fcb

Please sign in to comment.