[Webkit-unassigned] [Bug 25534] [GTK] Objects of ROLE_TABLE should implement the accessible table interface

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25534


Joanmarie Diggs <joanmarie.diggs at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41481|0                           |1
        is obsolete|                            |
  Attachment #41642|                            |review?
               Flag|                            |




--- Comment #5 from Joanmarie Diggs <joanmarie.diggs at gmail.com>  2009-10-22 00:21:56 PDT ---
Created an attachment (id=41642)
 --> (https://bugs.webkit.org/attachment.cgi?id=41642)
Part 1 - Take 2

(In reply to comment #4)
> So how does that work exactly? Does it simply put row after row, the index
> being n_columns*row + column or something like that?

Something very much like that. But we also have to keep cell spans in mind.
What we get from a traditional Gtk+ table which does not manage its descendants
is a table whose children are table cells, no intervening row or column
accessibles, indexed from 0 to childCount - 1.

> The way you are doing it
> in the patch you are simply relying on cells to be ordered the way you want, so
> I guess it wouldn't hurt to be a bit more explicit.

I added a bit more detail to the original comment. Please let me know if
further detail would be beneficial. Any concerns over the underlying order
changing in the vector down the road?

> if allCells is a normal C++ vector I think you can use std::find to get the
> index of the element intsead of looking for it manually. It will return an
> iterator, so to get the index value you just have to substract from it the
> first iterator in the vector (so... something like pos = find(allCells.begin(),
> allCells.end(), AXCell) - allCells.begin();)

Cool. Being new to C++, I love these sorts of tips. Thanks!!

> Now that I just wrote this I wonder if it actually matters or if the code is
> more readable that way... feel free to ignore it :)

Too late. I went with the change. :-)

> >+    return -1;
> 
> Don't think returning '-1' for error will work having a 'guint' return value :)

Merely a test to see if you were paying attention. Yeah... A test... Because...
I obviously was not. Thanks for catching this. Sorry it needed catching. Fixed.

> >+
> >+static gint webkit_accessible_table_get_column_extent_at(AtkTable* table, gint row, gint column)
> >+{
> >+    AccessibilityTableCell* AXCell = getCell(table, row, column);
> >+    if (AXCell) {
> >+        pair<int, int> columnRange;
> >+        AXCell->columnIndexRange(columnRange);
> >+        return columnRange.second;
> 
> 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?

The extent is indeed the span. Verified with testing with a variety of tables.

> I'm sure you'll figure it out in a follow-up patch ;)

Me too, as there really isn't a choice. :-) I've not quite figured it out yet,
however. If you'd like, I can just rip that function out until I get to the
bottom of it.

In addition to the above, this patch:

1. Renames a couple of private methods (fooBar instead of getFooBar) to be in
keeping with the style guide.

2. Implements:

   webkit_accessible_table_get_caption
   webkit_accessible_table_get_column_at_index
   webkit_accessible_table_get_row_at_index

   along with a new helper method, cellAtIndex.

3. Includes a "stub" for webkit_accessible_table_get_summary. (I thought it was
going to be pretty much like caption; I was wrong. If I shouldn't include such
stubs, let me know.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.