.
<rdar://problem/127671875>
Created attachment 471301 [details] Patch
Created attachment 471302 [details] Patch
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); ?
Created attachment 471308 [details] Patch
(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.
(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!
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.
Created attachment 471313 [details] Patch
Created attachment 471314 [details] Patch
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].