AX: Return AXEmptyGroup subrole for groups with no accessible content

• Component: Accessibility
• Status: RESOLVED FIXED

[Tyler Wilcock]

WebKit can calculate this more efficiently than AX clients.

[Radar WebKit Bug Importer]

<rdar://problem/84113795>

[Tyler Wilcock]

Created attachment 440830 [details]
WIP

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #2)
> Created attachment 440830 [details]
> WIP

--- a/Source/WebCore/accessibility/AccessibilityObjectInterface.h
+++ a/Source/WebCore/accessibility/AccessibilityObjectInterface.h

+    virtual bool hasAccessibleContent() const = 0;

I would call this hasContent. Everything here is already called "accessibility" or "accessible".

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp

+bool AccessibilityObject::hasAccessibleContent() const
+{
+    auto* first = firstChild();
+    if (first && first->accessibilityIsIgnored())
+        first = first->nextSiblingUnignored(30);

where is the 30 coming from?

If first is null, we shouldn't need to calculate last and we can return early.

+    auto* last = lastChild();
+    if (last && last->accessibilityIsIgnored())
+        last = last->previousSiblingUnignored(30);

30?

+    if (first == last && first->roleValue() == AccessibilityRole::StaticText) {
+        Vector<AccessibilityText> text;
+        accessibilityText(text);
+        // Consider this object empty if its only child has the same static text as its accessibility text.
+        if (text.size())
+            return text[0].text != first->stringValue();
+    }

We don't need to calculate first and last to determine that the object has just 1 child.

I would think that if we have a group with just one static text child, we should expose the static text object and not the group. So this doesn't make much sense to me.

[Tyler Wilcock]

Created attachment 440937 [details]
Patch

[Tyler Wilcock]

> I would think that if we have a group with just one static text child, we
> should expose the static text object and not the group. So this doesn't make
> much sense to me.

This is how it generally works. The difference is that this case tests the situation where a group is given an accessible label (i.e. via `title` attribute or `aria-label`). If the group's accessible label and the static text are the same, the static text becomes redundant.

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #5)
> > I would think that if we have a group with just one static text child, we
> > should expose the static text object and not the group. So this doesn't make
> > much sense to me.
> 
> This is how it generally works. The difference is that this case tests the
> situation where a group is given an accessible label (i.e. via `title`
> attribute or `aria-label`). If the group's accessible label and the static
> text are the same, the static text becomes redundant.

I would say the aria notation is redundant. I would favor the static text which is for example, selectable and visible on the screen, which the aria annotation is not.

[Tyler Wilcock]

> I would say the aria notation is redundant. I would favor the static text which is for example, selectable and visible on the screen, which the aria annotation is not.

Yeah, that makes sense. This seems like a separate bug, so we can handle that outside this patch.

[Tyler Wilcock]

Created attachment 441324 [details]
Patch

[Tyler Wilcock]

accessibility/mac/aria-grouping-roles.html will fail because of https://bugs.webkit.org/show_bug.cgi?id=231756. I'll retry the tests after that one lands.

[EWS]

Committed r284244 (243053@main): <https://commits.webkit.org/243053@main>

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

[WebKit Commit Bot]

Re-opened since this is blocked by bug 231813

*** Bug 231838 has been marked as a duplicate of this bug. ***

[Tyler Wilcock]

Created attachment 441496 [details]
Patch

[Tyler Wilcock]

I've re-run the iOS tests a bunch of times and the failures don't seem related. Most changes are also Mac-specific, so should be fine.

[EWS]

Committed r284387 (243169@main): <https://commits.webkit.org/243169@main>

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