[webkit-reviews] review granted: [Bug 271287] AX: Mac combo boxes should expose aria-activedescendant as AXSelectedChildren : [Attachment 470439] Patch

[bugzilla-daemon at webkit.org]

Tyler Wilcock <tyler_w at apple.com> has granted  review:
Bug 271287: AX: Mac combo boxes should expose aria-activedescendant as
AXSelectedChildren
https://bugs.webkit.org/show_bug.cgi?id=271287

Attachment 470439: Patch

https://bugs.webkit.org/attachment.cgi?id=470439&action=review




--- Comment #3 from Tyler Wilcock <tyler_w at apple.com> ---
Comment on attachment 470439
  --> https://bugs.webkit.org/attachment.cgi?id=470439
Patch

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

Thanks Dominic!

> LayoutTests/accessibility/mac/aria-combobox-activedescendant.html:11
> +<div role="list" id="list" aria-label="List1">
> +  <div role="listitem" id="item1" aria-label="item1">item1</div>

Are the id attributes on these elements used? If not, could make the test a bit
more concise by removing them?

> LayoutTests/accessibility/mac/aria-combobox-activedescendant.html:25
> +    var output = "On macOS, a combobox should map aria-activedescendant to
AXSelectedChildren.\n";
> +
> +    if (window.accessibilityController) {
> +	   var combobox =
accessibilityController.accessibleElementById("combobox");
> +	   output += expect("combobox.selectedChildrenCount", "1");
> +	   activeDescendant = combobox.selectedChildAtIndex(0);
> +	   output += expect("activeDescendant.role", "'AXRole: AXGroup'");
> +	   output += expect("activeDescendant.title", "'AXTitle: item2'");
> +	   debug(output);
> +    }

Minor nit, but our layout test style has been moving towards not indenting one
level inside script tags (so everything here can be shifted left).