[webkit-reviews] review requested: [Bug 129492] [ATK] Expose missing functionalities of AtkTableCell to AT. : [Attachment 225649] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 3 09:00:45 PST 2014


chris fleizach <cfleizach at apple.com> has asked	for review:
Bug 129492: [ATK] Expose missing functionalities of AtkTableCell to AT.
https://bugs.webkit.org/show_bug.cgi?id=129492

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

------- Additional Comments from chris fleizach <cfleizach at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225649&action=review


> Source/WebCore/ChangeLog:8
> +	   Implemented	missing API of AtkTableCell.

too many spaces after Implemented

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceTableCell.cpp:134
> +    AtkObject* table = atk_object_get_parent(axObject->wrapper());

should you verify that this is actually the table element before returning?

> Source/WebCore/accessibility/atk/WebKitAccessibleUtil.h:63
> +    AccessibilityObject* axObject = nullptr; \

It's a little non-intuitive that a local variable will be declared inside the
macro, that you reference outside the macro.

Maybe it would be better to write a function that does this same check

AccessibilityObject* axObject = verifyAccessibilityObjectIsValid(...);
if (!axObject)
return;


More information about the webkit-reviews mailing list