[webkit-reviews] review granted: [Bug 120552] AX: Expose DOM ID and ClassList to AX APIs for automation and AT element hashes : [Attachment 211122] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 120552: AX: Expose DOM ID and ClassList to AX APIs for automation and AT
element hashes
https://bugs.webkit.org/show_bug.cgi?id=120552

Attachment 211122: patch
https://bugs.webkit.org/attachment.cgi?id=211122&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=211122&action=review


Looks OK, but not building on most platforms.

Are you sure we want to export the class list as a single comma-separated
string?

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1508
> +static NSMutableArray *convertStringsToNSArray(const Vector<String>& vector)

> +{
> +    size_t length = vector.size();
> +    NSMutableArray *array = [NSMutableArray arrayWithCapacity:length];
> +    for (size_t i = 0; i < length; ++i)
> +	   [array addObject:vector[i]];
> +    return array;
> +}

Would be nice to share our implementation of this. There are lots of copies of
it in different classes in WebKit.

> Source/WebCore/accessibility/AccessibilityObject.h:548
> +    String domIdentifier() const;
> +    void domClassList(Vector<String>&) const;

Why the “dom” prefix on these?

> Source/WebCore/accessibility/AccessibilityObject.cpp:1654
> +    return getAttribute(idAttr);

fastGetAttribute