[webkit-reviews] review requested: [Bug 25534] [GTK] Objects of ROLE_TABLE should implement the accessible table interface : [Attachment 41959] table 2 - corrected as per review

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 27 10:08:37 PDT 2009


Joanmarie Diggs <joanmarie.diggs at gmail.com> has asked  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 41959: table 2 - corrected as per review
https://bugs.webkit.org/attachment.cgi?id=41959&action=review

------- Additional Comments from Joanmarie Diggs <joanmarie.diggs 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()).

D'oh! Pythonism. Fixed.

> >+		AccessibilityObject* accCell = allCells.at(index).get();
> >+		return static_cast<AccessibilityTableCell*>(accCell);
> >+	    }
> >+	}
> >+	return NULL;
> 
> While you are it, s/NULL/0/

Done.

Thanks for the review!


More information about the webkit-reviews mailing list