[webkit-reviews] review granted: [Bug 196601] [ATK] Cleanup accessible wrapper base class : [Attachment 366803] Rebased patch

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 196601: [ATK] Cleanup accessible wrapper base class
https://bugs.webkit.org/show_bug.cgi?id=196601

Attachment 366803: Rebased patch

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




--- Comment #4 from Mario Sanchez Prada <mario at webkit.org> ---
Comment on attachment 366803
  --> https://bugs.webkit.org/attachment.cgi?id=366803
Rebased patch

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

It looks good to me, only one comment on that there's a part of the code that
looks like could be removed altogether (either now or in a follow-up patch)

>
Source/WebCore/accessibility/atk/WebKitAccessible.cppSource/WebCore/accessibili
ty/atk/WebKitAccessibleWrapperAtk.cpp:369
> +	   auto* atkParent = parent ? ATK_OBJECT(parent->wrapper()) : nullptr;

It seems like this will always yield nullptr to atkParent, since `parent ==
nullptr` already if you made it here... meaning that the for loop below won't
ever be executed.

Actually, it seems that the whole `(!parent && isRootObject(coreObject)) { ...
}` block can be removed, as all we can do if `parent == nullptr` at this point
is to return -1, which is what the if block right after this one does.

However, I think it's better to keep this for now as this patch is just about
cleaning things up and translating, so feel free to address that in a follow-up
patch.