Closed Bug 423224 Opened 12 years ago Closed 12 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: MarcoZ, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access, dev-doc-complete, Whiteboard: orca:urgent)

Attachments

(1 file)

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?
Flags: blocking1.9?
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.
Blocks: orca
Whiteboard: orca:urgent
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.
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?
(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?
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.
Target Milestone: mozilla1.9beta5 → mozilla1.9
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Given we are past RC1 freeze and there has been no bug activity this has to wait until a dot release...
Flags: wanted1.9.0.x?
Flags: blocking1.9-
Flags: blocking1.9+
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.
Exposing a cell-number object attribute is certainly a low risk fix. I agree we should take it.
Attached patch patchSplinter Review
modified mochitest fails on cellRefAt but it's different issue which we should have a deal in another bug with.
Attachment #315239 - Flags: review?(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"
Attachment #315239 - Flags: review?(Evan.Yan) → review+
(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
Status: NEW → ASSIGNED
Attachment #315239 - Flags: approval1.9?
Attachment #315239 - Flags: approval1.9? → approval1.9+
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.
Summary: Accessible hierarchies different in Windows and Linux for tables, causes getIndexInParent for table cells to return bogus values on Windows. → Expose object attribute on cell accessible for html:td to provide cell index
/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

Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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!
Status: RESOLVED → VERIFIED
Keywords: dev-doc-needed
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.
No, that is fine, thanks!
Flags: wanted1.9.0.x?
You need to log in before you can comment on or make changes to this bug.