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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 20 02:47:15 PDT 2009


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


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41481|                            |review-
               Flag|                            |




--- Comment #4 from Xan Lopez <xan.lopez at gmail.com>  2009-10-20 02:47:15 PDT ---
(From update of attachment 41481)
>+
>+static guint getCellIndex(AccessibilityTableCell* AXCell, AccessibilityTable* AXTable)
>+{
>+    // Calculate the cell's index as if we had traditional Gtk+ tables.

So how does that work exactly? Does it simply put row after row, the index
being n_columns*row + column or something like that? 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.

>+    AccessibilityObject::AccessibilityChildrenVector allCells;
>+    AXTable->cells(allCells);
>+    unsigned cellCount = allCells.size();
>+    for (unsigned k = 0; k < cellCount; ++k)
>+        if (allCells[k] == AXCell)
>+            return k;

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();)

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 :)

>+    return -1;

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

>+}
>+
>+static AtkObject* webkit_accessible_table_ref_at(AtkTable* table, gint row, gint column)
>+{
>+    AccessibilityTableCell* AXCell = getCell(table, row, column);
>+    if (!AXCell)
>+        return NULL;
>+    return AXCell->wrapper();
>+}
>+
>+static gint webkit_accessible_table_get_index_at(AtkTable* table, gint row, gint column)
>+{
>+    AccessibilityTableCell* AXCell = getCell(table, row, column);
>+    AccessibilityTable* AXTable = static_cast<AccessibilityTable*>(core(table));
>+    return getCellIndex(AXCell, AXTable);
>+}
>+
>+static gint webkit_accessible_table_get_n_columns(AtkTable* table)
>+{
>+    AccessibilityObject* accTable = core(table);
>+    if (accTable->isAccessibilityRenderObject())
>+        return static_cast<AccessibilityTable*>(accTable)->columnCount();
>+    return 0;
>+}
>+
>+static gint webkit_accessible_table_get_n_rows(AtkTable* table)
>+{
>+    AccessibilityObject* accTable = core(table);
>+    if (accTable->isAccessibilityRenderObject())
>+        return static_cast<AccessibilityTable*>(accTable)->rowCount();
>+    return 0;
>+}
>+
>+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? If the function returns the extent shouldn't you return second -
first?

>+    }
>+    return 0;
>+}
>+
>+static gint webkit_accessible_table_get_row_extent_at(AtkTable* table, gint row, gint column)
>+{
>+    AccessibilityTableCell* AXCell = getCell(table, row, column);
>+    if (AXCell) {
>+        pair<int, int> rowRange;
>+        AXCell->rowIndexRange(rowRange);
>+        return rowRange.second;
>+    }
>+    return 0;
>+}
>+
>+static AtkObject* webkit_accessible_table_get_column_header(AtkTable* table, gint column)
>+{
>+    // Something is not quite right here. :-( We get headers but not the child
>+    // text which holds the header content. Also note that the nearly identical
>+    // get_row_header works as expected. Might be related to the funky hierarchy.
>+    AccessibilityObject* accTable = core(table);
>+    if (accTable->isAccessibilityRenderObject()) {
>+        AccessibilityObject::AccessibilityChildrenVector allColumnHeaders;
>+        static_cast<AccessibilityTable*>(accTable)->columnHeaders(allColumnHeaders);
>+
>+        unsigned colCount = allColumnHeaders.size();
>+        for (unsigned k = 0; k < colCount; ++k) {
>+            AccessibilityObject* columnObject = allColumnHeaders[k]->parentObject();
>+            if (static_cast<AccessibilityTableColumn*>(columnObject)->columnIndex() == column)
>+                return allColumnHeaders[k]->wrapper();
>+        }
>+    }
>+    return NULL;
>+}

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

>+
>+static AtkObject* webkit_accessible_table_get_row_header(AtkTable* table, gint row)
>+{
>+    AccessibilityObject* accTable = core(table);
>+    if (accTable->isAccessibilityRenderObject()) {
>+        AccessibilityObject::AccessibilityChildrenVector allRowHeaders;
>+        static_cast<AccessibilityTable*>(accTable)->rowHeaders(allRowHeaders);
>+
>+        unsigned rowCount = allRowHeaders.size();
>+        for (unsigned k = 0; k < rowCount; ++k) {
>+            AccessibilityObject* rowObject = allRowHeaders[k]->parentObject();
>+            if (static_cast<AccessibilityTableRow*>(rowObject)->rowIndex() == row)
>+                return allRowHeaders[k]->wrapper();
>+        }
>+    }
>+    return NULL;
>+}
>+
>+static void atk_table_interface_init(AtkTableIface* iface)
>+{
>+    iface->ref_at = webkit_accessible_table_ref_at;
>+    iface->get_index_at = webkit_accessible_table_get_index_at;
>+    iface->get_n_columns = webkit_accessible_table_get_n_columns;
>+    iface->get_n_rows = webkit_accessible_table_get_n_rows;
>+    iface->get_column_extent_at = webkit_accessible_table_get_column_extent_at;
>+    iface->get_row_extent_at = webkit_accessible_table_get_row_extent_at;
>+    iface->get_column_header = webkit_accessible_table_get_column_header;
>+    iface->get_row_header = webkit_accessible_table_get_row_header;
>+}
>+
> static const GInterfaceInfo AtkInterfacesInitFunctions[] = {
>     {(GInterfaceInitFunc)atk_action_interface_init,
>      (GInterfaceFinalizeFunc) NULL, NULL},
>@@ -991,6 +1124,8 @@ static const GInterfaceInfo AtkInterfacesInitFunctions[] = {
>     {(GInterfaceInitFunc)atk_component_interface_init,
>      (GInterfaceFinalizeFunc) NULL, NULL},
>     {(GInterfaceInitFunc)atk_image_interface_init,
>+     (GInterfaceFinalizeFunc) NULL, NULL},
>+    {(GInterfaceInitFunc)atk_table_interface_init,
>      (GInterfaceFinalizeFunc) NULL, NULL}
> };
> 
>@@ -999,7 +1134,8 @@ enum WAIType {
>     WAI_EDITABLE_TEXT,
>     WAI_TEXT,
>     WAI_COMPONENT,
>-    WAI_IMAGE
>+    WAI_IMAGE,
>+    WAI_TABLE
> };
> 
> static GType GetAtkInterfaceTypeFromWAIType(WAIType type)
>@@ -1015,6 +1151,8 @@ static GType GetAtkInterfaceTypeFromWAIType(WAIType type)
>       return ATK_TYPE_COMPONENT;
>   case WAI_IMAGE:
>       return ATK_TYPE_IMAGE;
>+  case WAI_TABLE:
>+      return ATK_TYPE_TABLE;
>   }
> 
>   return G_TYPE_INVALID;
>@@ -1048,6 +1186,10 @@ static guint16 getInterfaceMaskFromObject(AccessibilityObject* coreObject)
>     if (coreObject->isImage())
>         interfaceMask |= 1 << WAI_IMAGE;
> 
>+    // Table
>+    if (role == TableRole)
>+        interfaceMask |= 1 << WAI_TABLE;
>+
>     return interfaceMask;
> }
> 
>-- 
>1.6.3.3
>

Looks great to me but for those small comments, let's fix them and commit it.

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



More information about the webkit-unassigned mailing list