Open Bug 576838 Opened 9 years ago Updated 3 years ago

nsIAccessibleTable is broken for crazy tables

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

REOPENED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 4 open bugs)

Details

(Keywords: access, Whiteboard: [approved-patches-landed])

Attachments

(1 file)

<div style="display:table;" id="table6">
    <input type="checkbox">
    <a href="bar">Bad checkbox</a>
  </div>

div is table accessible and GetCellAt(0, 0) called on it returns div itself. Row and column count is 1.
Attached patch tmp patchSplinter Review
it prevents Orca hangs but Orca is somehow borken. Any way it's better than we have now. Later we should get real fix.
Attachment #455931 - Flags: review?(marco.zehe)
Attachment #455931 - Flags: review?(bolterbugz)
Blocks: orca
Comment on attachment 455931 [details] [diff] [review]
tmp patch

r=me.
Attachment #455931 - Flags: review?(bolterbugz) → review+
Comment on attachment 455931 [details] [diff] [review]
tmp patch

r=me thanks!
Attachment #455931 - Flags: review?(marco.zehe) → review+
asking for blocking since this makes Orca hang while Orca processes crazy HTML tables because firefox exposes wrong information.
blocking2.0: --- → ?
Bernd, could you look at this bug please? The table from bug description has one row and one column and GetCellDataAt() of nsITableLayout returns div element (a table itself) for (0, 0) cell.
Comment on attachment 455931 [details] [diff] [review]
tmp patch

Major for Orca screen reader, trivial fix.
Attachment #455931 - Flags: approval2.0?
Attachment #455931 - Flags: approval2.0? → approval2.0+
blocking2.0: ? → final+
Assignee: nobody → surkov.alexander
http://hg.mozilla.org/mozilla-central/rev/24e5ac894021
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
bug isn't fixed still, table interface is broken, the patch prevents hangs of Orca, nothing else.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #9)
> bug isn't fixed still, table interface is broken, the patch prevents hangs of
> Orca, nothing else.

We should decide whether we should keep this bug in release blocking status. Orca is still broken with this patch and patch from bug 420993 (I think because of broken nsIAccessibleTable interface). Once bug 420993 is fixed then we should ask Marco/Joanie for the help to define this bug majority.
To be honest, with "Everything Else Going On," I've not yet had time to test this change. (Sorry!) However, if it stops a hang in Orca, I for one would love to see it retain it's blocking status. :-)
Ah I just read the last few comments after filing bug 587529. Yes I agree nsIAccessibleTable appears broken in some way and I'm fine with unblocking here since we really should do deeper investigating/fixing on a separate bug IMO.
Joanie, could try nightly firefox build and say how the current behavior affects on orca please?
I'm still seeing some navigation issues, which I've not yet had time to dig deeper into. And I still see at least one instance of a dead accessible as I commented in bug 420993. However, things are improving. :-)

Thanks Alex!
OK, Fernando is going to re-triage this one to assess blocking status.
blocking2.0: final+ → .x
So this is not an issue anymore with orca as we have a workaround. Maybe we should open a bug against layout because of these tables returning themselves when querying the content.
(In reply to comment #16)
> So this is not an issue anymore with orca as we have a workaround. Maybe we
> should open a bug against layout because of these tables returning themselves
> when querying the content.

We have a wallpaper fix in a11y code so we should get back it once real problem is fixed. I think it's ok to keep this bug for both issues.

But actually we should get feedback from table layout people like Bernd.
>But actually we should get feedback from table layout people like Bernd.
What is the question?
(In reply to comment #18)
> >But actually we should get feedback from table layout people like Bernd.
> What is the question?

See comment #6. My concern is GetCellDataAt should never return table itself.
For anonymous table frame creation you should not expect that  nsTableFrame::GetCellDataAt will return in side the nsIDomElement a HTMLTableCellElement

The correct text in nsHTMLTableAccessible.cpp would be

Due to anonymous table frame creation we are not guaranteed that this will be the real table cell content, it can also be the content which is responsible for the need to create a cell frame.  

instead of referencing "crazy tables"
(In reply to comment #20)

> Due to anonymous table frame creation we are not guaranteed that this will be
> the real table cell content, it can also be the content which is responsible
> for the need to create a cell frame.  

Can be the layout fixed so we are guaranteed if there's a cell frame then there's its own cell content? Since accessible tree is created based on DOM tree then we get a problem here because there's no cell accessible while it should be.
Blocks: a11ynext
Blocks: treea11y
IMHO, your requirement does not match what http://www.w3.org/TR/CSS2/tables.html#anonymous-boxes requires and what we implement.
you will always have cell frames with out a dom table cell object
(In reply to comment #22)
> IMHO, your requirement does not match what
> http://www.w3.org/TR/CSS2/tables.html#anonymous-boxes requires and what we
> implement.

they said "anonymous boxes in visual table layout" but does it deny us to create native anonymous content for this? If Gecko allows the following:
table
  native anonymous row
   native anonymous cell
     explicit child

but iirc it doesn't allow this (XBL does but that's not a case for XBL usage I think). If that's correct then perhaps we should fix nsIContent::GetChildren() method for tables making it crawl through frame tree rather than DOM tree or think of different way of accessible tree creation.
Whiteboard: [approved-patches-landed]
Is there a way we can cope with comment 23 on the a11y side?
(In reply to comment #25)
> Is there a way we can cope with comment 23 on the a11y side?

see comment #24.
Sounds like we don't know.
I mean we don't know the right way to go forward. (?)
Boris will definitely know if anonymous content nodes are possible /feasible/desirable. For me the answer to all 3 options is no. But that is a half informed guess.
Anonymous content nodes corresponding to anonymous child frames would require a good bit of surgery on DOM and layout code (think wholesale changes to the way anonymous content and things like parent nodes work) and aren't really desirable, in my opinion, even if they were feasible.  I'm not at all sure they're feasible, by the way; I suspect trying to do that would be a multi-year bug-hunting project.

If accessibles need to represent pure-layout information not directly tied to content, then it seems like we need to allow accessibles to exist that map to a particular frame, even if it's not the primary frame for a node.
Hmm.  I guess we could do something "xbl like", but I'm not sure it's worth it still...
(In reply to comment #30)
> If accessibles need to represent pure-layout information not directly tied to
> content, then it seems like we need to allow accessibles to exist that map to a
> particular frame, even if it's not the primary frame for a node.

I agree, that's what needs to happen here.

nsIAccessible doesn't seem to require that there be one nsIAccessible per DOM node, so it should be fine to construct one nsIAccessible per table part frame.
However, you don't want to have a frame pointer stored persistently in your accessibles, that would be bad.

I see we don't have nsIAccessibleTableRow/RowGroup/Column. That simplifies the problem.

Maybe we could define a new nsAnonymousTableCellAccessible which just stores its parent (table) accessible and a row/column index. getTableCellAt would return an nsIAccessible for the table cell element if there is one, otherwise it would find or construct a nsAnonymousTableCellAccessible and return it.

Alternatively, the nsAnonymousTableCellAccessible could store a reference to the first child node of the cell.

Both of these alternatives would break in different ways under some kinds of DOM mutations. So would alternatives that hold onto a frame pointer, though. I'm not sure there's a reasonable way to track the identity of anonymous cells through all kinds of DOM manipulation. I'd probably just go with the row/column index approach.
(which would be relatively simple to implement, and would work perfectly in all cases except where someone inserts or removes a table row or column before an anonymous table-cell, while the AT is using the cell's nsIAccessible.)
Alex do you still need this bug open?
Flags: needinfo?(surkov.alexander)
(In reply to David Bolter [:davidb] from comment #35)
> Alex do you still need this bug open?

I think this is a valid issue still.
Flags: needinfo?(surkov.alexander)
You need to log in before you can comment on or make changes to this bug.