1486668 - <td> with display: block doesn't get table cell semantics

Status: RESOLVED FIXED

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

Description

[James Teh [:Jamie]]

In bug 1005271, we fixed <table> with CSS style display: block so they get table semantics for accessibility. Unfortunately, it seems there are tables in the wild where most of the table is styled normally, but individual cells are styled with display: block.

STR (with NVDA):
1. Open this test case:
data:text/html,<table><tr><th>a</th><td style="display: block;">b</td></tr></table>
2. Focus the document and press control+end to move to the last line.
3. Press control+alt+left to move to the previous column.
Expected: NVDA says "column 1  a"
Actual: NVDA says "not in a table cell"

Impact: This can be seen in the real world when viewing Gitlab diff views such as this one:
https://t.co/b5sCw4Ifni
The cells in the column containing the actual lines of code are all styled with display: block. This makes it impossible to efficiently navigate between lines of code.

Comment 1

[Marco Zehe (:MarcoZ)]

Attachment: WIP

Alex, this is a WIP patch for this bug. The test passes, but on the page Jamie links to in comment #0, NVDA skips to the first cell of the next row if I go to the cell with the code and then press CTRL+Alt+DownArrow to move down to the next cell _in the same column_. I don't get row and column headers, other parts of the nsIAccessibleTable interface seem to work, like Cell-Index etc. Any pointers would be appreciated where I could check. Is the general approach reasonable here?

Jamie, I assume the reason why NVDA can't go o the same column on the next row is because some value isn't being returned properly.

Comment 2

[James Teh [:Jamie]]

I'm not Alex :), but I did some investigation on this yesterday, since I thought we could take a similar approach, so I thought I'd comment. Unfortunately, it's not so simple... but respect for diving in and taking a stab at it nevertheless. :)

What this patch does is effectively the same as putting role="cell" on the cell with display: block:
data:text/html,<table><tr><th>a</th><td style="display: block;" role="cell">b</td></tr></table>
At first glance, this does seem to work: the cell gets the right role, interface and coordinates. Unfortunately, when you ask the table to give you the cell at these coordinates, it returns... the row. That causes all sorts of breakage for NVDA.

The reason is that even though the cell is an ARIAGridCellAccessible, the table is still an HTMLTableAccessible. HTMLTableAccessible::CellAt uses DOM interfaces to retrieve the requested cell. But because this isn't a real table cell according to the DOM, that doesn't work properly. Why it returns the row instead of failing is beyond me, but there it is.

I think what we're going to need to do here is have HTMLTableAccessible fall back to the ARIAGridAccessible behaviour if the DOM returns something other than a cell. That means moving even more of ARIAGridAccessible into the base TableAccessible class.

There's a further issue which we may choose to address in another bug. However, I mention it here because it serves to demonstrate what's going on here behind the scenes. Consider this test case:
data:text/html,<table><tr><th colspan="2">a</th><td style="display: block;" role="cell">b</td></tr></table>
The first cell occupies 2 columns. However, notice that the second cell reports as column 2, when it should really be column 3. That's because ARIAGridCellAccessible is being used, and it doesn't account for HTML table cell colspan. To fix this one, ARIAGridCellAccessible needs to increment for column spans when it is iterating through table cells.

Comment 3

[alexander :surkov (:asurkov)]

As Jamie mentioned the main problem is HTMLTableAccessible relies on layout to implement AccessibleTable interface. If HTML:td has display:cell, then it's no longer part of HTML table. I'm not 100% sure whether it has to be a cell semantically or not, but the problem remains if you put @role="cell" on the td element.

So, for the original testcase, I suggest to reach HTML-AAM group and browser vendors to make sure we stay aligned. For role="cell" example data:text/html,<table><tr><th>a</th><td role="cell" style="display: block;">b</td></tr></table>, we should try to stick with what Jamie suggested, i.e. try to use ARIAGridAccessible behavior as fallback for HTMLTableAccessible. It might be not easy though.

Comment 4

[James Teh [:Jamie]]

(In reply to alexander :surkov (:asurkov) from comment #3)
> So, for the original testcase, I suggest to reach HTML-AAM group and browser
> vendors to make sure we stay aligned.

Given that we now do table semantics for display: block (bug 1005271), I think doing the same for <td> makes sense for consistency if nothing else.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #4)
> (In reply to alexander :surkov (:asurkov) from comment #3)
> > So, for the original testcase, I suggest to reach HTML-AAM group and browser
> > vendors to make sure we stay aligned.
> 
> Given that we now do table semantics for display: block (bug 1005271), I
> think doing the same for <td> makes sense for consistency if nothing else.

I can see your point. However td having no display:table-cell is a tricky one, since it may affect on table processing model and also makes things complicated :) There's a couple of examples of the past where Firefox was picking a "correct" approach, while other vendors rolled out for simplicity.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 9004912 [details] [diff] [review]
WIP

Review of attachment 9004912 [details] [diff] [review]:
-----------------------------------------------------------------

forgot to cancel feedback per comment 2, comment 3

Comment 7

[Marco Zehe (:MarcoZ)]

Attachment: Bug 1486668 - <td> with display: block doesn't get table cell semantics, r=surkov

Comment 8

[Marco Zehe (:MarcoZ)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c554d92a6e8b89589cfc7da1bca6a70abad3d0c2

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 9009973 [details]
Bug 1486668 - <td> with display: block doesn't get table cell semantics, r=surkov

alexander :surkov (:asurkov) has approved the revision.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/001c36e011c4
<td> with display: block doesn't get table cell semantics, r=surkov

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/001c36e011c4