[webkit-reviews] review denied: [Bug 25534] [GTK] Objects of ROLE_TABLE should implement the accessible table interface : [Attachment 41913] table 2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 27 05:47:48 PDT 2009
Xan Lopez <xan.lopez at gmail.com> has denied Joanmarie Diggs
<joanmarie.diggs at gmail.com>'s request for review:
Bug 25534: [GTK] Objects of ROLE_TABLE should implement the accessible table
interface
https://bugs.webkit.org/show_bug.cgi?id=25534
Attachment 41913: table 2
https://bugs.webkit.org/attachment.cgi?id=41913&action=review
------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>+static AccessibilityTableCell* cellAtIndex(AtkTable* table, gint index)
>+{
>+ AccessibilityObject* accTable = core(table);
>+ if (accTable->isAccessibilityRenderObject()) {
>+ AccessibilityObject::AccessibilityChildrenVector allCells;
>+ static_cast<AccessibilityTable*>(accTable)->cells(allCells);
>+ if (0 <= index < allCells.size()) {
Mmm, I don't think this does what you think. '0 <= index' becomes a boolean
(0/1), which is then compared against allCells.size(). So you'd get TRUE in
cases like 0 <= 1000 < 10, because 0 <= 1000 -> 1, 1 < 10 -> 1.
What you need is simply if (0 <= index && index < allCells.size()).
>+ AccessibilityObject* accCell = allCells.at(index).get();
>+ return static_cast<AccessibilityTableCell*>(accCell);
>+ }
>+ }
>+ return NULL;
While you are it, s/NULL/0/
Marking r- for now, but looks good to me otherwise.
More information about the webkit-reviews
mailing list