Bug 171178 - AX: Treat cells with ARIA table cell properties as cells
Summary: AX: Treat cells with ARIA table cell properties as cells
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joanmarie Diggs (irc: joanie)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-22 17:16 PDT by Joanmarie Diggs (irc: joanie)
Modified: 2017-05-03 10:18 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.43 KB, patch)
2017-05-03 09:16 PDT, Joanmarie Diggs (irc: joanie)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs (irc: joanie) 2017-04-22 17:16:14 PDT
This was discovered as part of the ARIA 1.1 CR testing. The failures are due to the following not being exposed as a table:

  <table>
    <tr>
      <td aria-colspan="2">test cell</td>
    </tr>
  </table>

This strikes me as yet another case of "Why on earth would anyone do that?" And admittedly, this is an artificial/quick-and-dirty test case. That said, if an author has taken the time to use an ARIA table cell property on a table cell, I think we have expose the cell as a cell and the containing table as a table.
Comment 1 Radar WebKit Bug Importer 2017-04-22 17:16:47 PDT
<rdar://problem/31775750>
Comment 2 Joanmarie Diggs (irc: joanie) 2017-05-03 09:16:21 PDT
Created attachment 308909 [details]
Patch
Comment 3 chris fleizach 2017-05-03 09:24:47 PDT
Comment on attachment 308909 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308909&action=review

> Source/WebCore/accessibility/AccessibilityTable.cpp:160
> +    if (ariaColumnCount == -1 || ariaColumnCount > 0)

is if (!ariaColumnCount) sufficient here? or do you care if the colcount was set to -2?

> Source/WebCore/accessibility/AccessibilityTable.cpp:-169
> -    if (numRows == 1 && numCols == 1)

we may want to move this to the end. so if none of the rows or cols had the aria attributes, and we have only 1 cell...
Comment 4 Joanmarie Diggs (irc: joanie) 2017-05-03 09:38:45 PDT
Comment on attachment 308909 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308909&action=review

>> Source/WebCore/accessibility/AccessibilityTable.cpp:160
>> +    if (ariaColumnCount == -1 || ariaColumnCount > 0)
> 
> is if (!ariaColumnCount) sufficient here? or do you care if the colcount was set to -2?

I do care. If the colcount is set to -2, the author has provided an invalid value. If we are going to respect the author's desire to expose their minimalist table as a data table, I think they should at least do us the courtesy of providing a valid value. ;)

Do you think we should instead accept any non-zero value?

>> Source/WebCore/accessibility/AccessibilityTable.cpp:-169
>> -    if (numRows == 1 && numCols == 1)
> 
> we may want to move this to the end. so if none of the rows or cols had the aria attributes, and we have only 1 cell...

I am happy to make that change, but in that particular case the existing heuristic concludes it is not a data table. See the last table in the layout test. The td element with id of "cell8" is an AXGroup on your platform and an ATK_ROLE_SECTION on mine.
Comment 5 chris fleizach 2017-05-03 09:45:22 PDT
Comment on attachment 308909 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308909&action=review

>>> Source/WebCore/accessibility/AccessibilityTable.cpp:160
>>> +    if (ariaColumnCount == -1 || ariaColumnCount > 0)
>> 
>> is if (!ariaColumnCount) sufficient here? or do you care if the colcount was set to -2?
> 
> I do care. If the colcount is set to -2, the author has provided an invalid value. If we are going to respect the author's desire to expose their minimalist table as a data table, I think they should at least do us the courtesy of providing a valid value. ;)
> 
> Do you think we should instead accept any non-zero value?

probably not

>>> Source/WebCore/accessibility/AccessibilityTable.cpp:-169
>>> -    if (numRows == 1 && numCols == 1)
>> 
>> we may want to move this to the end. so if none of the rows or cols had the aria attributes, and we have only 1 cell...
> 
> I am happy to make that change, but in that particular case the existing heuristic concludes it is not a data table. See the last table in the layout test. The td element with id of "cell8" is an AXGroup on your platform and an ATK_ROLE_SECTION on mine.

ok
Comment 6 WebKit Commit Bot 2017-05-03 10:18:46 PDT
Comment on attachment 308909 [details]
Patch

Clearing flags on attachment: 308909

Committed r216123: <http://trac.webkit.org/changeset/216123>
Comment 7 WebKit Commit Bot 2017-05-03 10:18:48 PDT
All reviewed patches have been landed.  Closing bug.