Bug 196601 - [ATK] Cleanup accessible wrapper base class
Summary: [ATK] Cleanup accessible wrapper base class
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: Gtk, InRadar
Depends on:
Reported: 2019-04-04 08:02 PDT by Carlos Garcia Campos
Modified: 2019-04-08 01:11 PDT (History)
4 users (show)

See Also:

Patch (66.30 KB, patch)
2019-04-04 08:11 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (66.30 KB, patch)
2019-04-05 00:37 PDT, Carlos Garcia Campos
mario: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2019-04-04 08:02:11 PDT
Comment 1 Radar WebKit Bug Importer 2019-04-04 08:03:11 PDT
Comment 2 Carlos Garcia Campos 2019-04-04 08:11:49 PDT
Created attachment 366713 [details]
Comment 3 Carlos Garcia Campos 2019-04-05 00:37:11 PDT
Created attachment 366803 [details]
Rebased patch
Comment 4 Mario Sanchez Prada 2019-04-05 03:00:42 PDT
Comment on attachment 366803 [details]
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/accessibility/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.
Comment 5 Carlos Garcia Campos 2019-04-08 01:11:26 PDT
Committed r243970: <https://trac.webkit.org/changeset/243970>