Bug 273826 – AX: When a text input field is contained within an element with role combobox, the input field should inherit the role combobox.

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Andres Gonzalez]

.

[Radar WebKit Bug Importer]

<rdar://problem/127671875>

[Andres Gonzalez]

Created attachment 471301 [details]
Patch

[Andres Gonzalez]

Created attachment 471302 [details]
Patch

[Tyler Wilcock]

Comment on attachment 471302 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=471302&action=review

The other case we may want to consider is when a text field dynamically gains a combobox ancestor that it should inherit the role from. e.g., maybe the wrapping div doesn't initially have role="combobox", but gains it. Do we handle that correctly in this patch already?

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:520
> +        auto* combobox = Accessibility::findAncestor<AccessibilityObject>(*this, false, [&] (const auto& ancestor) {
> +            AXLOG(ancestor);
> +            return ancestor.isComboBox();
> +        });

Rather than a full ancestry traversal, I wonder if we should stop at the nearest non-generic and non-group? i.e., we probably don't want a textfield to inherit the combobox role if the combobox ancestor is 300 levels away with other roles (table, list, etc) in between. This would be more performant, too. Also, should we keep this AXLOG(ancestor); ?

[Andres Gonzalez]

Created attachment 471308 [details]
Patch

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #4)
> Comment on attachment 471302 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=471302&action=review
> 
> The other case we may want to consider is when a text field dynamically
> gains a combobox ancestor that it should inherit the role from. e.g., maybe
> the wrapping div doesn't initially have role="combobox", but gains it. Do we
> handle that correctly in this patch already?

AG: I considered this but would like to see a real relevant case where this is happening before we extend this accommodation:
1. this will involve a traversal of the subtree searching for the potential input element, which it could be expensive.
2. the present change is to accommodate a practice recommended against after  ARIA 1.1. Thus it seems to me unnecessary to extend this accommodation for a legacy practice unless there is a compelling case beyond a theoretical possibility.


> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:520
> > +        auto* combobox = Accessibility::findAncestor<AccessibilityObject>(*this, false, [&] (const auto& ancestor) {
> > +            AXLOG(ancestor);
> > +            return ancestor.isComboBox();
> > +        });
> 
> Rather than a full ancestry traversal, I wonder if we should stop at the
> nearest non-generic and non-group? i.e., we probably don't want a textfield
> to inherit the combobox role if the combobox ancestor is 300 levels away
> with other roles (table, list, etc) in between. This would be more
> performant, too. Also, should we keep this AXLOG(ancestor); ?

Addressed.

[Tyler Wilcock]

(In reply to Andres Gonzalez from comment #6)
> (In reply to Tyler Wilcock from comment #4)
> > Comment on attachment 471302 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=471302&action=review
> > 
> > The other case we may want to consider is when a text field dynamically
> > gains a combobox ancestor that it should inherit the role from. e.g., maybe
> > the wrapping div doesn't initially have role="combobox", but gains it. Do we
> > handle that correctly in this patch already?
> 
> AG: I considered this but would like to see a real relevant case where this
> is happening before we extend this accommodation:
> 1. this will involve a traversal of the subtree searching for the potential
> input element, which it could be expensive.
> 2. the present change is to accommodate a practice recommended against after
> ARIA 1.1. Thus it seems to me unnecessary to extend this accommodation for a
> legacy practice unless there is a compelling case beyond a theoretical
> possibility.
Agreed, thanks!

[Tyler Wilcock]

Comment on attachment 471308 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=471308&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:516
> +        // Text fields may have a combobox ancestor, in which case we want to return role combobox.

Sorry for missing this on the first round, but it might be nice to mention that this was the ARIA 1.1 combobox pattern that has since been deprecated, but is still used by some sites. Just so future us don't think this was an adhoc decision to support this style of combobox.

[Andres Gonzalez]

Created attachment 471313 [details]
Patch

[Andres Gonzalez]

Created attachment 471314 [details]
Patch

[EWS]

Committed 278506@main (1c70b1ecc16a): <https://commits.webkit.org/278506@main>

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