[Webkit-unassigned] [Bug 57463] [GTK] ARIA tables not exposing cells as HTML tables do

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Mario Sanchez Prada <msanchez at igalia.com>  2011-04-01 10:55:43 PST ---
(In reply to comment #3)
> (From update of attachment 87839 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=87839&action=review
> 
> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:113
> > +
> 
> !child check is not needed

Ok

> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:115
> > +            if (appendedRows.contains(row))
> 
> this check could be in the previous check. there's no need to cast to a AccessibilityTableRow at this point

Ok

> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:119
> > +            unsigned rowCellCount = row->children().size();
> 
> Store instead of "store"

Ok, oops.

> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:123
> > +            row->setRowIndex((int)columnCount);
> 
> static_cast
> also i don't think the rowIndex is the same as the columnCount. The row index should be m_rows.size()

That was a mistake. Thanks for the catch.

> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:125
> > +            m_children.append(row->children());
> 
> why is all this logic duplicated from addChild. Is there any difference with what you added?

The main difference is that in addChild() your adding always rows (as it's claimed by the assertion in the first line), but here in this patch we're adding the row's children instead, since we don't want (for the time being at least) those row objects to be exposed to ATK, but just their children (the cells).

That is, we want the a11y objects for the cells to be exposed as direct children of the a11y objects for the tables. No a11y objects for the rows in the middle, for consistency with GTK (and with what it's done for HTML tables).

What is true is that perhaps I could refactor code in a better way, maybe through an extra parameter in addChild() so we avoid this code replication.


> > Source/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:450
> > +    case GridRole:
> 
> i believe you removed this in another patch

True, sorry.

Now I'm running out of time but will try to work on fixing this patch asap, if possible before Monday.

Thanks for the feedback

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