[Webkit-unassigned] [Bug 169924] AX: WebKit should not expose redundant AXGroups with missing role when the label is the same as the contents

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=169924

--- Comment #8 from Tyler Wilcock <tyler_w at apple.com> ---
> +                if (nodeDocument->childNeedsStyleRecalc())
> +                    return String();
>
> return { };

There is code two lines above that also does `return String();`. I can change both if that's the style preference here.

> +    // Never ignore a <div> with event listeners attached to it (e.g.
> onclick).
> +    if (axObject.node() && axObject.node()->hasEventListeners())
> +        return false;
> 
> Isn't this check done for any object already in the isIgnored method?
> Wondering why we need to do this specifically for groups.

If it is, I can't find it. It probably should exist everywhere?

Regardless, I think we'd still need this check to avoid returning a false positive for a group with redundant AX text and an event handler.

> +    auto* first = axObject.firstChild();
> +    if (first && first == axObject.lastChild() && first->roleValue() ==
> AccessibilityRole::StaticText) {
> 
> Wouldn't this be more efficient?
> 
> +    auto& children = axObject.children();
> +    if (children.size() == 1 && children[0]->roleValue() ==
> AccessibilityRole::StaticText) {
> 
> or the problem is that this is used in children()?

We can't call children() here because children() is not const (it updates the children if required). Making any "is ignored" function non-const will make many other things have to be non-const as well.

This is generally unfortunate, because it makes ignoring things based on their contents much more difficult. We'll have to comb through the AX objects represented by the raw layout tree ourselves. It works fine in this simple example, but more complex analyses will be harder and fragile.

> +        Vector<AccessibilityText> text;
> ...
> +        auto axText = text.size() ? text[0].text : String();
> 
> Aren't these names inverted?  i.e., shouldn't the String be text and the
> vector axText?
> 
> What if the vector has more than one element? Perhaps we should return false
> in that case.

My understanding of accessibilityText is that it's ordered in terms of the most applicable text based on https://www.w3.org/TR/accname-1.2/#step1. So by picking the first one, we are choosing the one AX clients should be using when describing the element. Web Inspector also picks the first accessibilityText.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211014/5b9096e7/attachment-0001.htm>