- Revision
- 284245
- Author
- [email protected]
- Date
- 2021-10-15 07:35:04 -0700 (Fri, 15 Oct 2021)
Log Message
AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents
https://bugs.webkit.org/show_bug.cgi?id=169924
Patch by Tyler Wilcock <[email protected]> on 2021-10-15
Reviewed by Andres Gonzalez.
Source/WebCore:
Don't expose groups with redundant accessibility text on the Mac.
Here's an example of one such group:
<div aria-label="1">1</div>
This group is not useful to accessibility clients. Instead, we
expose the text contents directly.
Test: accessibility/ignore-redundant-accessibility-text-groups.html
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::textUnderElement const):
Instead of asserting when we try to get the text of an element in a
document that needs a style update, return an empty string.
* accessibility/mac/AccessibilityObjectMac.mm:
(WebCore::shouldIgnoreGroup): Added.
(WebCore::AccessibilityObject::accessibilityPlatformIncludesObject const):
LayoutTests:
Add a test ensuring WebKit appropriately does and doesn't expose
groups with redundant accessibility text.
* accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt: Added.
* accessibility/mac/ignore-redundant-accessibility-text-groups.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (284244 => 284245)
--- trunk/LayoutTests/ChangeLog 2021-10-15 14:32:36 UTC (rev 284244)
+++ trunk/LayoutTests/ChangeLog 2021-10-15 14:35:04 UTC (rev 284245)
@@ -1,5 +1,18 @@
2021-10-15 Tyler Wilcock <[email protected]>
+ AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents
+ https://bugs.webkit.org/show_bug.cgi?id=169924
+
+ Reviewed by Andres Gonzalez.
+
+ Add a test ensuring WebKit appropriately does and doesn't expose
+ groups with redundant accessibility text.
+
+ * accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt: Added.
+ * accessibility/mac/ignore-redundant-accessibility-text-groups.html: Added.
+
+2021-10-15 Tyler Wilcock <[email protected]>
+
AX: Return AXEmptyGroup subrole for groups with no accessible content
https://bugs.webkit.org/show_bug.cgi?id=231528
Added: trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt (0 => 284245)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt 2021-10-15 14:35:04 UTC (rev 284245)
@@ -0,0 +1,30 @@
+This test ensures WebKit ignores groups with redundant accessibility text.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS !ariaLabelGroup is true
+PASS !titleGroup is true
+PASS !ariaLabelDiv is true
+PASS !titleDiv is true
+PASS typeof clickHandlerGroup is 'object'
+PASS typeof clickHandlerDiv is 'object'
+PASS resultElement.role is 'AXRole: AXStaticText'
+PASS resultElement.stringValue is 'AXValue: Blue cheese'
+PASS resultElement.role is 'AXRole: AXStaticText'
+PASS resultElement.stringValue is 'AXValue: Oranges'
+PASS contentContainer.childrenCount is 6
+PASS contentContainer.childAtIndex(0).stringValue is 'AXValue: Blue cheese'
+PASS contentContainer.childAtIndex(0).role is 'AXRole: AXStaticText'
+PASS contentContainer.childAtIndex(1).stringValue is 'AXValue: Oranges'
+PASS contentContainer.childAtIndex(1).role is 'AXRole: AXStaticText'
+PASS contentContainer.childAtIndex(2).role is 'AXRole: AXGroup'
+PASS contentContainer.childAtIndex(3).stringValue is 'AXValue: Jello'
+PASS contentContainer.childAtIndex(3).role is 'AXRole: AXStaticText'
+PASS contentContainer.childAtIndex(4).stringValue is 'AXValue: Broccoli'
+PASS contentContainer.childAtIndex(4).role is 'AXRole: AXStaticText'
+PASS contentContainer.childAtIndex(5).role is 'AXRole: AXGroup'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html (0 => 284245)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html (rev 0)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html 2021-10-15 14:35:04 UTC (rev 284245)
@@ -0,0 +1,94 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+
+<div id="content" role="group">
+ <!-- First, test role="group" elements. -->
+ <div id="aria-label-group" role="group" aria-label="Blue cheese">
+ Blue cheese
+ </div>
+
+ <div id="title-group" role="group" title="Oranges">
+ Oranges
+ </div>
+
+ <div id="click-handler-group" role="group" aria-label="Group click handler" _onclick_="emptyClickHandler()">
+ Group click handler
+ </div>
+
+ <!-- Also test role-less generic divs. -->
+ <div id="aria-label-div" aria-label="Jello">
+ Jello
+ </div>
+
+ <div id="title-div" title="Broccoli">
+ Broccoli
+ </div>
+
+ <div id="click-handler-div" aria-label="Div click handler" _onclick_="emptyClickHandler()">
+ Div click handler
+ </div>
+</div>
+
+<script>
+ description("This test ensures WebKit ignores groups with redundant accessibility text.");
+ const emptyClickHandler = () => {};
+
+ if (window.accessibilityController) {
+
+ var contentContainer = accessibilityController.accessibleElementById("content");
+
+ var ariaLabelGroup = accessibilityController.accessibleElementById("aria-label-group");
+ var clickHandlerGroup = accessibilityController.accessibleElementById("click-handler-group");
+ var titleGroup = accessibilityController.accessibleElementById("title-group");
+
+ var ariaLabelDiv = accessibilityController.accessibleElementById("aria-label-div");
+ var clickHandlerDiv = accessibilityController.accessibleElementById("click-handler-div");
+ var titleDiv = accessibilityController.accessibleElementById("title-div");
+
+ // We shouldn't be able to get an accessible element for these groups because they should be ignored.
+ shouldBeTrue("!ariaLabelGroup");
+ shouldBeTrue("!titleGroup");
+ shouldBeTrue("!ariaLabelDiv");
+ shouldBeTrue("!titleDiv");
+ // But any group with an event handler should always be exposed.
+ shouldBe("typeof clickHandlerGroup", "'object'");
+ shouldBe("typeof clickHandlerDiv", "'object'");
+
+ // Ensure we can search for the text within the ignored groups.
+ var resultElement = contentContainer.uiElementForSearchPredicate(contentContainer, true, "AXAnyTypeSearchKey", "", false);
+ shouldBe("resultElement.role", "'AXRole: AXStaticText'");
+ shouldBe("resultElement.stringValue", "'AXValue: Blue cheese'");
+
+ resultElement = contentContainer.uiElementForSearchPredicate(resultElement, true, "AXAnyTypeSearchKey", "", false);
+ shouldBe("resultElement.role", "'AXRole: AXStaticText'");
+ shouldBe("resultElement.stringValue", "'AXValue: Oranges'");
+
+ // Ensure the only accessible content exposed via `children` is the text elements and event handler groups.
+ shouldBe("contentContainer.childrenCount", "6");
+ shouldBe("contentContainer.childAtIndex(0).stringValue", "'AXValue: Blue cheese'");
+ shouldBe("contentContainer.childAtIndex(0).role", "'AXRole: AXStaticText'");
+
+ shouldBe("contentContainer.childAtIndex(1).stringValue", "'AXValue: Oranges'");
+ shouldBe("contentContainer.childAtIndex(1).role", "'AXRole: AXStaticText'");
+
+ shouldBe("contentContainer.childAtIndex(2).role", "'AXRole: AXGroup'");
+
+ shouldBe("contentContainer.childAtIndex(3).stringValue", "'AXValue: Jello'");
+ shouldBe("contentContainer.childAtIndex(3).role", "'AXRole: AXStaticText'");
+
+ shouldBe("contentContainer.childAtIndex(4).stringValue", "'AXValue: Broccoli'");
+ shouldBe("contentContainer.childAtIndex(4).role", "'AXRole: AXStaticText'");
+
+ shouldBe("contentContainer.childAtIndex(5).role", "'AXRole: AXGroup'");
+
+ document.getElementById("content").style.visibility = "hidden";
+ }
+</script>
+</body>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (284244 => 284245)
--- trunk/Source/WebCore/ChangeLog 2021-10-15 14:32:36 UTC (rev 284244)
+++ trunk/Source/WebCore/ChangeLog 2021-10-15 14:35:04 UTC (rev 284245)
@@ -1,5 +1,31 @@
2021-10-15 Tyler Wilcock <[email protected]>
+ AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents
+ https://bugs.webkit.org/show_bug.cgi?id=169924
+
+ Reviewed by Andres Gonzalez.
+
+ Don't expose groups with redundant accessibility text on the Mac.
+ Here's an example of one such group:
+
+ <div aria-label="1">1</div>
+
+ This group is not useful to accessibility clients. Instead, we
+ expose the text contents directly.
+
+ Test: accessibility/ignore-redundant-accessibility-text-groups.html
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::textUnderElement const):
+ Instead of asserting when we try to get the text of an element in a
+ document that needs a style update, return an empty string.
+
+ * accessibility/mac/AccessibilityObjectMac.mm:
+ (WebCore::shouldIgnoreGroup): Added.
+ (WebCore::AccessibilityObject::accessibilityPlatformIncludesObject const):
+
+2021-10-15 Tyler Wilcock <[email protected]>
+
AX: Return AXEmptyGroup subrole for groups with no accessible content
https://bugs.webkit.org/show_bug.cgi?id=231528
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (284244 => 284245)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-10-15 14:32:36 UTC (rev 284244)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-10-15 14:35:04 UTC (rev 284245)
@@ -680,12 +680,14 @@
if (Frame* frame = nodeDocument->frame()) {
// catch stale WebCoreAXObject (see <rdar://problem/3960196>)
if (frame->document() != nodeDocument)
- return String();
+ return { };
- // Renders referenced by accessibility objects could get destroyed, if TextIterator ends up triggering
- // style update/layout here. See also AXObjectCache::deferTextChangedIfNeeded().
- ASSERT_WITH_SECURITY_IMPLICATION(!nodeDocument->childNeedsStyleRecalc());
+ // Renderers referenced by accessibility objects could get destroyed if TextIterator ends up triggering
+ // a style update or layout here. See also AXObjectCache::deferTextChangedIfNeeded().
+ if (nodeDocument->childNeedsStyleRecalc())
+ return { };
ASSERT_WITH_SECURITY_IMPLICATION(!nodeDocument->view()->layoutContext().isInRenderTreeLayout());
+
return plainText(*textRange, textIteratorBehaviorForTextRange());
}
}
Modified: trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm (284244 => 284245)
--- trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm 2021-10-15 14:32:36 UTC (rev 284244)
+++ trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm 2021-10-15 14:35:04 UTC (rev 284245)
@@ -114,6 +114,28 @@
return true;
}
+static bool shouldIgnoreGroup(const AccessibilityObject& axObject)
+{
+ if (!axObject.isGroup() && axObject.roleValue() != AccessibilityRole::Div)
+ return false;
+
+ // Never ignore a <div> with event listeners attached to it (e.g. onclick).
+ if (axObject.node() && axObject.node()->hasEventListeners())
+ return false;
+
+ auto* first = axObject.firstChild();
+ if (first && first == axObject.lastChild() && first->roleValue() == AccessibilityRole::StaticText) {
+ Vector<AccessibilityText> axText;
+ axObject.accessibilityText(axText);
+ // Don't expose <div>s whose only child is text that has the same content as the <div>s accessibility text.
+ // Instead, we should expose the text element directly.
+ auto firstText = axText.size() ? axText[0].text : String();
+ if (first->stringValue() == firstText)
+ return true;
+ }
+ return false;
+}
+
AccessibilityObjectInclusion AccessibilityObject::accessibilityPlatformIncludesObject() const
{
if (isMenuListPopup() || isMenuListOption())
@@ -142,6 +164,9 @@
}
}
+ if (shouldIgnoreGroup(*this))
+ return AccessibilityObjectInclusion::IgnoreObject;
+
return AccessibilityObjectInclusion::DefaultBehavior;
}
