WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
231528
AX: Return AXEmptyGroup subrole for groups with no accessible content
https://bugs.webkit.org/show_bug.cgi?id=231528
Summary
AX: Return AXEmptyGroup subrole for groups with no accessible content
Tyler Wilcock
Reported
2021-10-11 13:35:00 PDT
WebKit can calculate this more efficiently than AX clients.
Attachments
WIP
(12.00 KB, patch)
2021-10-11 13:39 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Patch
(13.45 KB, patch)
2021-10-12 08:42 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Patch
(20.71 KB, patch)
2021-10-14 20:12 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Patch
(23.21 KB, patch)
2021-10-16 10:53 PDT
,
Tyler Wilcock
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-10-11 13:35:16 PDT
<
rdar://problem/84113795
>
Tyler Wilcock
Comment 2
2021-10-11 13:39:45 PDT
Created
attachment 440830
[details]
WIP
Andres Gonzalez
Comment 3
2021-10-12 07:54:20 PDT
(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
Comment 4
2021-10-12 08:42:16 PDT
Created
attachment 440937
[details]
Patch
Tyler Wilcock
Comment 5
2021-10-12 08:46:02 PDT
> 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
Comment 6
2021-10-12 08:52:39 PDT
(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
Comment 7
2021-10-12 09:09:16 PDT
> 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
Comment 8
2021-10-14 20:12:36 PDT
Created
attachment 441324
[details]
Patch
Tyler Wilcock
Comment 9
2021-10-14 20:15:01 PDT
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
Comment 10
2021-10-15 07:32:43 PDT
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
Comment 11
2021-10-15 09:56:29 PDT
Re-opened since this is blocked by
bug 231813
ayumi_kojima
Comment 12
2021-10-15 15:28:58 PDT
***
Bug 231838
has been marked as a duplicate of this bug. ***
Tyler Wilcock
Comment 13
2021-10-16 10:53:30 PDT
Created
attachment 441496
[details]
Patch
Tyler Wilcock
Comment 14
2021-10-18 10:42:34 PDT
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
Comment 15
2021-10-18 11:55:35 PDT
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]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug