[webkit-reviews] review granted: [Bug 127688] AX: Support @scope in HTML tables : [Attachment 222387] patch

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 127688: AX: Support @scope in HTML tables
https://bugs.webkit.org/show_bug.cgi?id=127688

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

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=222387&action=review


Assuming that the Objective-C bits are Ok (which do seem Ok to me, with the
little knowledge I have about the language), the patch looks good to me.

I would only ask you for uploading a new version (no need to review it again)
of the patch removing the uiElementArrayAttributeValue() empty stub from WKTR's
AccessibilityUIElementAtk.cpp before landing it, so we make sure that such a
change is enough to make EWS bots happy.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:-93
> -#ifndef NSAccessibilityRowHeaderUIElementsAttribute
> -#define NSAccessibilityRowHeaderUIElementsAttribute @"AXRowHeaderUIElements"

> -#endif
> -

I guess this is intentional? Just double checking

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:828
> +JSValueRef AccessibilityUIElement::uiElementArrayAttributeValue(JSStringRef
attribute) const
> +{
> +    // FIXME: implement
> +    return nullptr;
> +}
> +    

You don't need to add this empty method, it's already there (and the reason why
it's failing to build in GTK and EFL)