| Summary: | [ATK] Cleanup accessible wrapper base class | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
| Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bugs-noreply, jdiggs, mario, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | Gtk, InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Carlos Garcia Campos
2019-04-04 08:02:11 PDT
Created attachment 366713 [details]
Patch
Created attachment 366803 [details]
Rebased patch
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. Committed r243970: <https://trac.webkit.org/changeset/243970> |