small improvement.

Comment on attachment 376847 [details] [diff] [review] patch2 >+ /** >+ * Query nsAccessNode from the given nsIAccessNode. >+ */ >+ static already_AddRefed<nsHTMLTableAccessible> >+ QueryAccessibleTable(nsIAccessibleTable *aAccessibleTable); The description for this function is completely bogus. :-) It should read something like "Query the nsIAccessibleTable interface from the given accessible." >+nsresult >+nsHTMLTableHeaderAccessible::GetRoleInternal(PRUint32 *aRole) >+{ >+ nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode)); >+ >+ // Check value of @scope attribute. >+ if (content->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::scope, >+ nsAccessibilityAtoms::col, eIgnoreCase)) { >+ *aRole = nsIAccessibleRole::ROLE_COLUMNHEADER; >+ return NS_OK; >+ } >+ >+ if (content->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::scope, >+ nsAccessibilityAtoms::row, eIgnoreCase)) { Would it perhaps be more performant if we queried that attrValue once, store it in a local variable and then simply compare that against the atoms? It strikes me as not right to do this query twice. >+ /** >+ * >+ */ >+ nsresult FindCellsForRelation(PRInt32 aSearchHint, PRUint32 aRelationType, >+ nsIAccessibleRelation **aRelation); >+ >+ /** >+ * >+ */ >+ nsIContent* FindCell(nsHTMLTableAccessible *aTableAcc, nsIContent *aAnchorCell, >+ PRInt32 aRowIdx, PRInt32 aColIdx, >+ PRInt32 aLookForHeader); >+}; Nit: Add proper comments please. >+ <tobdy> Typo :-) >+ <td id="table2_3" scope="col">col1</th> Wrong closing tag, should be </td>. The other cell below that has it correct.

Comment on attachment 376879 [details] [diff] [review] patch The tests look great. I also don't see anything else in the patch that sticks out at me, but leaving that to davidb and neil for r and sr. r=me.

A drive by concern is around performance. Walking dom for each TD is a bit scary no? I guess this work will help treegrid support?

(In reply to comment #7) > A drive by concern is around performance. Walking dom for each TD is a bit > scary no? I guess this work will help treegrid support? David, thinking more I think it's not scary because we do more ugly walking through DOMs when we calculate description_for relations from aria-describedby. So it shouldn't hit us. Since this algorithm is proposed by HTML 4.1 spec then it's worth to follow it until we meet performance problem.

Comment on attachment 376879 [details] [diff] [review] patch OK thanks, r=me

Is our table/grid/etc work going to be landed on 1.9.1?

(In reply to comment #10) > Is our table/grid/etc work going to be landed on 1.9.1? As far as I understood Rich Schwerdtfeger, no, it suffices for Firefox 3.next.

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090517 Minefield/3.6a1pre (.NET CLR 3.5.30729)