RESOLVED FIXED Bug 196601
[ATK] Cleanup accessible wrapper base class
https://bugs.webkit.org/show_bug.cgi?id=196601
Summary [ATK] Cleanup accessible wrapper base class
Carlos Garcia Campos
Reported 2019-04-04 08:02:11 PDT
.
Attachments
Patch (66.30 KB, patch)
2019-04-04 08:11 PDT, Carlos Garcia Campos
no flags
Rebased patch (66.30 KB, patch)
2019-04-05 00:37 PDT, Carlos Garcia Campos
mario: review+
Radar WebKit Bug Importer
Comment 1 2019-04-04 08:03:11 PDT
Carlos Garcia Campos
Comment 2 2019-04-04 08:11:49 PDT
Carlos Garcia Campos
Comment 3 2019-04-05 00:37:11 PDT
Created attachment 366803 [details] Rebased patch
Mario Sanchez Prada
Comment 4 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.
Carlos Garcia Campos
Comment 5 2019-04-08 01:11:26 PDT
Note You need to log in before you can comment on or make changes to this bug.