[webkit-reviews] review granted: [Bug 29672] fix accessibility webkit-style-check errors : [Attachment 42358] patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 29672: fix accessibility webkit-style-check errors
https://bugs.webkit.org/show_bug.cgi?id=29672

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

------- Additional Comments from David Levin <levin at chromium.org>
I know there are mixed feelings about style clean up, but I think it is nice to
have a consistent style in the code, so I'll review it.

It would be nice to fix up a few nits before check-in.

> Index: WebCore/accessibility/AXObjectCache.h

> +struct TextMarkerData  {

Might as well fix the two spaces before the {. (Make it one space.)

> +    bool nodeIsAriaType(Node* node, String role);

No need for the param name "node" here.

> Index: WebCore/accessibility/AccessibilityObject.cpp
>      for (parent = parentObject(); parent &&
parent->accessibilityIsIgnored(); parent = parent->parentObject())
> +    { }

The preferred style would be to have the open { right after the ) of the for
statement. I would think the close brace would go on the next line.


> Index: WebCore/accessibility/AccessibilityRenderObject.cpp

> @@ -296,10 +296,10 @@ bool AccessibilityRenderObject::isSlider
>  bool AccessibilityRenderObject::isMenuRelated() const
>  {
>      AccessibilityRole role = roleValue();
> -    return  role == MenuRole ||
> -	       role == MenuBarRole ||
> -	       role == MenuButtonRole ||
> -	       role == MenuItemRole;
> +    return  role == MenuRole 
> +	       || role == MenuBarRole
> +	       || role == MenuButtonRole
> +	       || role == MenuItemRole;

There are two spaces after the return. Also, if you only indent the subsequent
lines by 4 spaces, things line up nicely (much like before):

return role == MenuRole 
    || role == MenuBarRole
    || role == MenuButtonRole
    || role == MenuItemRole;