[webkit-reviews] review granted: [Bug 62718] [GTK] Consider rows being ignored when adding children to tables : [Attachment 98358] Patch proposal + unskipped layout test

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 62718: [GTK] Consider rows being ignored when adding children to tables
https://bugs.webkit.org/show_bug.cgi?id=62718

Attachment 98358: Patch proposal + unskipped layout test
https://bugs.webkit.org/attachment.cgi?id=98358&action=review

------- Additional Comments from chris fleizach <cfleizach at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=98358&action=review

r=me with suggested fixes

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:309
> +

looks like you could put this after the !coreParent check, then you wouldn't
need to check coreParent in this if statement

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:319
> +    unsigned tableChildrenCount = tableChildren.size();

i think this unsigned should be a size_t, along with cellsCount (size
vector.size() returns size_t i believe)

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:323
> +    for (unsigned i = 0; i < tableChildrenCount; ++i) {

size_t for the i as well

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:352
> +

size_t change

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:358
> +	       if (static_cast<unsigned>(index) < cellsCount +
rowChildrenCount)

you could probably static_cast<index> one time before the loop, so it doesn't
have to be done everytime within the loop

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:403
> +    AccessibilityObject* grandParent = parent->parentObjectUnignored();

you should probably verify parent != 0 before getting gradparent

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:410
> +

size_t

> Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:418
> +

size_t