423224 - Expose object attribute on cell accessible for html:td to provide cell index

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[Marco Zehe (:MarcoZ)]

This is a spin-off off bug 416742. To recap, here's what Joanie and I discovered (see also that bug's comment 48). The original testcase is a table with a caption, one row and two cells in that row.

On Linux, the hierarchy looks like this:

table
  caption
  cell 0
  cell 1

The testcase fails.

On Windows, the IAccessible2 hierarchy, viewed in AccProbe, looks like this:

table
  caption
    text leaf of caption
  textframe
    table cell 0
      text leaf of table cell 0
  textframe
    table cell 1
      text leaf of table cell 1

On Windows, the testcase always is true, because getIndexInParent always
returns 0, because the only child of the textframe parent *is* the table cell.

So, why are we creating textframes in IAccessible2 for each table cell while we
don't do it on Linux where aparently there is no such thing as textframes?

Comment 1

[Aaron Leventhal]

The hierarchies can be different than expected on Linux as well, if a <tr>, <tbody>, <thead> or <tfoot> is focusable or has something else "interesting" about it like an ARIA property. In that case we have no choice but to expose the extra object. The table impl has to deal with that.

Comment 2

[Joanmarie Diggs]

Marco, thanks for filing this bug and for spending the time at CSUN to go over this stuff with me!  Question:  Looking at the summary "... bogus values on Windows", I was wondering if this bug will also address the original problem I filed in bug 416742 or if we should open a new bug for that issue.

Comment 3

[alexander :surkov (:asurkov)]

We may want to have similar accessibles trees for table but we can't guarantee as Aaron said the cell accessibles only are presented in the table. Therefore getIndexInParent() and cell index are different things. If we will expose 'cell-index' attribute on cell then will it help?

Comment 4

[Marco Zehe (:MarcoZ)]

(In reply to comment #3)
> If we will expose
> 'cell-index' attribute on cell then will it help?

Joanie, in other words, if we gave you the cell-based index in an attribute for the cell, would that be good for Orca so you can then query the row and columns by getRowAtIndex and getColumnAtIndex?

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

TM --> mozilla1.9, this won't block beta 5. If you disagree, please reset the TM to beta5 and explain why it needs to block beta.

Comment 6

[Mike Schroepfer]

Given we are past RC1 freeze and there has been no bug activity this has to wait until a dot release...

Comment 7

[Marco Zehe (:MarcoZ)]

We have an idea of an interim fix, which is giving Orca the actual cell number it needs to ask the table for, in an attribute string. That would, for now, work around the problem of GetIndexInParent and the reverse lookup of GetColumnAtIndex and GetRowAtIndex not working consistently in HTML tables.

Alexander, since nobody responded to either the newsgroup or on the Wiki page where you proposed the solution possibilities, please move forward with implementing the cellNumber attribute string that contains the index of the cell.

Mike, I request that this be taken for FX3 when ready, because this not being able to give users the accurate column and row numbers is a major table reading problem on Linux. Things like speaking the correct column header stands and falls with this implementation problem. So please allow us to give the screen reader this workaround to work with.

Comment 8

[Aaron Leventhal]

Exposing a cell-number object attribute is certainly a low risk fix. I agree we should take it.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch

modified mochitest fails on cellRefAt but it's different issue which we should have a deal in another bug with.

Comment 10

[Evan Yan]

Comment on attachment 315239 [details] [diff] [review]
patch

Some nits:

>-ACCESSIBILITY_ATOM(lineNumber, "line-number")
>-ACCESSIBILITY_ATOM(containerRelevant, "container-relevant")
>-ACCESSIBILITY_ATOM(containerLive, "container-live")
>-ACCESSIBILITY_ATOM(containerChannel, "container-channel")
>-ACCESSIBILITY_ATOM(containerAtomic, "container-atomic")
>-ACCESSIBILITY_ATOM(containerBusy, "container-busy")

Why we move these?

>+  while (parentAcc) {
>+    if (Role(parentAcc) == nsIAccessibleRole::ROLE_TABLE) {
>+      // Table accessible must implement nsIAccessibleTable interface but if
>+      // it isn't happen (for example because of ARIA usage) we shouldn't fail
>+      // another attributes on table cell.

I guess you mean "we shouldn't fail on getting other attributes"

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> (From update of attachment 315239 [details] [diff] [review])
> Some nits:
> 
> >-ACCESSIBILITY_ATOM(lineNumber, "line-number")
> >-ACCESSIBILITY_ATOM(containerRelevant, "container-relevant")
> >-ACCESSIBILITY_ATOM(containerLive, "container-live")
> >-ACCESSIBILITY_ATOM(containerChannel, "container-channel")
> >-ACCESSIBILITY_ATOM(containerAtomic, "container-atomic")
> >-ACCESSIBILITY_ATOM(containerBusy, "container-busy")
> 
> Why we move these?

to keep them in alphabetical order like we do in nsAccessibilityAtoms

Comment 12

[alexander :surkov (:asurkov)]

changing summary. If we really want to keep trees in sync on different platforms (that should be nice of course imo) then let me know and I'll file new bug to track this.

Comment 13

[alexander :surkov (:asurkov)]

/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <--  nsAccessibilityAtomList.h
new revision: 1.91; previous revision: 1.90
done
Checking in accessible/src/html/nsHTMLTableAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTableAccessible.cpp,v  <--  nsHTMLTableAccessible.cpp
new revision: 1.62; previous revision: 1.61
done
Checking in accessible/src/html/nsHTMLTableAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLTableAccessible.h,v  <--  nsHTMLTableAccessible.h
new revision: 1.33; previous revision: 1.32
done
Checking in accessible/tests/mochitest/test_table_indexes.html;
/cvsroot/mozilla/accessible/tests/mochitest/test_table_indexes.html,v  <--  test_table_indexes.html
new revision: 1.2; previous revision: 1.1
done

Comment 14

[Marco Zehe (:MarcoZ)]

Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041506 Minefield/3.0pre.

Alexander, can you document this on our Wiki (how to use the cell-index attribute instead of GetIndexInParent? Thanks!

Comment 15

[alexander :surkov (:asurkov)]

Marco, I put already some documentation at http://developer.mozilla.org/en/docs/Accessibility:AT-APIs:Gecko:Attrs#Table_related_attributes. If you think it should be improved let me know.

Comment 16

[Marco Zehe (:MarcoZ)]

No, that is fine, thanks!