[webkit-reviews] review requested: [Bug 25534] [GTK] Objects of ROLE_TABLE should implement the accessible table interface : [Attachment 41898] table 1 (breaking these apart for easier review)

[bugzilla-daemon at webkit.org]

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 41898: table 1 (breaking these apart for easier review)
https://bugs.webkit.org/attachment.cgi?id=41898&action=review

------- Additional Comments from Joanmarie Diggs <joanmarie.diggs at gmail.com>
It's rough being the new kid. ;-)

Okay. This is based on the original patch reviewed by Xan. Changes:

1. Makes the suggested (and then withdrawn) change to use std:find.

2. Correct the guint/gint error

3. Acknowledges, but makes no changes in response to, this comment:

> Just a sanity check: what comes in that pair, the indexes of the columns the
> cell occupies?

Nope. It's the index of the column and then the column span:

void AccessibilityTableCell::columnIndexRange(pair<int, int>& columnRange)
{
    if (!m_renderer || !m_renderer->isTableCell())
	return;

    RenderTableCell* renderCell = toRenderTableCell(m_renderer);
    columnRange.first = renderCell->col();
    columnRange.second = renderCell->colSpan();    
}

> If the function returns the extent shouldn't you return second -
> first?

I need the colSpan, columnRange.second contains the colSpan. Testing with
tables with multiple spans would also seem to indicate that what I have done is
sound.

4. Removes webkit_accessible_table_get_column_header. Xan suggests I'll figure
it out in a follow-up patch. Agreed. Might as well just introduce it into the
code then.

Please review. Thanks!