Bug 148967 - AX: ARIA 1.1 implement aria-colcount, aria-colindex, aria-colspan, aria-rowcount, aria-rowindex and aria-rowspan
Summary: AX: ARIA 1.1 implement aria-colcount, aria-colindex, aria-colspan, aria-rowco...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 163644 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-09-08 10:50 PDT by Nan Wang
Modified: 2016-12-13 23:43 PST (History)
13 users (show)

See Also:


Attachments
patch (49.34 KB, patch)
2015-10-09 12:37 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (49.86 KB, patch)
2015-10-09 13:14 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (50.11 KB, patch)
2015-10-09 15:24 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
example html (1.13 KB, text/html)
2015-10-16 13:55 PDT, Nan Wang
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2015-09-08 10:50:13 PDT
Added support for ARIA 1.1 table/grid related attributes.

aria-colcount
http://www.w3.org/TR/wai-aria-1.1/#aria-colcount
Defines an element's column index or position with respect to the total number of columns within a table, grid, or treegrid. 

aria-colindex
http://www.w3.org/TR/wai-aria-1.1/#aria-colindex
Defines an element's column index or position with respect to the total number of columns within a table, grid, or treegrid. S

aria-colspan
http://www.w3.org/TR/wai-aria-1.1/#aria-colspan
Defines the number of columns spanned by a cell or gridcell within a table, grid, or tree grid.

aria-rowcount
http://www.w3.org/TR/wai-aria-1.1/#aria-rowcount
Defines the total number of rows in a table, grid, or tree grid.

aria-rowindex
http://www.w3.org/TR/wai-aria-1.1/#aria-rowindex
Defines an element's row index or position with respect to the total number of rows within a table, grid, or tree grid.

aria-rowspan
http://www.w3.org/TR/wai-aria-1.1/#aria-rowspan
Defines the number of rows spanned by a cell or gridcell within a table, grid, or treegrid.
Comment 1 Radar WebKit Bug Importer 2015-09-08 10:50:46 PDT
<rdar://problem/22612833>
Comment 2 Nan Wang 2015-10-09 12:37:55 PDT
Created attachment 262787 [details]
patch

I don't want to mess up with the native table attributes/properties (like cell index ranges, table column/row counts, etc.), so added several new attributes for mac to query for these new table/grid related values. 

On mac, maybe VoiceOver can check these new attributes first then decide what to speak.
Comment 3 Nan Wang 2015-10-09 13:14:04 PDT
Created attachment 262789 [details]
patch

Fixed iOS build failure.
Comment 4 chris fleizach 2015-10-09 14:12:03 PDT
Comment on attachment 262789 [details]
patch

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

whats the plan for exposing this on iOS?

> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:96
> +    unsigned rowSpan = ariaRowSpan();

how do we distinguish 0 from no value?

> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:101
> +        rowSpan = 1;

can you put this blob into a separate method

> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:145
> +}

i know that column/rowIndex are used by VO to get data out of the table sometimes. will this mess things up?

> Source/WebCore/accessibility/AccessibilityTable.cpp:695
> +    int rowCountInt = rowCountValue.toInt();

if there's no aria_rowCount set, won't toInt() == 0? which seems like the wrong default value right

> Source/WebCore/accessibility/AccessibilityTableRow.cpp:152
> +    for (unsigned i = 0; i < rowChildren.size(); i++) {

are we able to use the better iterators for accessibility children

for (const auto& cell in rowChildren)
Comment 5 Nan Wang 2015-10-09 14:37:49 PDT
Comment on attachment 262789 [details]
patch

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

For iOS I'm thinking of adding new methods to expose those values. I might open a new bug for iOS and tests. But seems iOS is not speaking table row/column count or indexes now.

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:96
>> +    unsigned rowSpan = ariaRowSpan();
> 
> how do we distinguish 0 from no value?

If no value, the function will return 1 as default.

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:101
>> +        rowSpan = 1;
> 
> can you put this blob into a separate method

Ok

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:145
>> +}
> 
> i know that column/rowIndex are used by VO to get data out of the table sometimes. will this mess things up?

This is only for non-native tables, so I think the colspan/rowspan attributes won't be affected from this code. If author didn't set aria-colspan/aria-rowspan, the range.second will be 1 as before.

>> Source/WebCore/accessibility/AccessibilityTable.cpp:695
>> +    int rowCountInt = rowCountValue.toInt();
> 
> if there's no aria_rowCount set, won't toInt() == 0? which seems like the wrong default value right

My idea is to return the value only if the aria-rowcount is greater than the rendered table row count. Otherwise just return -1, and VO doesn't have to worry about speaking it.
The spec says: authors must set the value of aria-rowcount to -1 to indicate that the value should not be calculated by the user agent. Not sure if we should do something special about it.

>> Source/WebCore/accessibility/AccessibilityTableRow.cpp:152
>> +    for (unsigned i = 0; i < rowChildren.size(); i++) {
> 
> are we able to use the better iterators for accessibility children
> 
> for (const auto& cell in rowChildren)

Ok
Comment 6 Nan Wang 2015-10-09 14:37:50 PDT
Comment on attachment 262789 [details]
patch

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

For iOS I'm thinking of adding new methods to expose those values. I might open a new bug for iOS and tests. But seems iOS is not speaking table row/column count or indexes now.

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:96
>> +    unsigned rowSpan = ariaRowSpan();
> 
> how do we distinguish 0 from no value?

If no value, the function will return 1 as default.

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:101
>> +        rowSpan = 1;
> 
> can you put this blob into a separate method

Ok

>> Source/WebCore/accessibility/AccessibilityARIAGridCell.cpp:145
>> +}
> 
> i know that column/rowIndex are used by VO to get data out of the table sometimes. will this mess things up?

This is only for non-native tables, so I think the colspan/rowspan attributes won't be affected from this code. If author didn't set aria-colspan/aria-rowspan, the range.second will be 1 as before.

>> Source/WebCore/accessibility/AccessibilityTable.cpp:695
>> +    int rowCountInt = rowCountValue.toInt();
> 
> if there's no aria_rowCount set, won't toInt() == 0? which seems like the wrong default value right

My idea is to return the value only if the aria-rowcount is greater than the rendered table row count. Otherwise just return -1, and VO doesn't have to worry about speaking it.
The spec says: authors must set the value of aria-rowcount to -1 to indicate that the value should not be calculated by the user agent. Not sure if we should do something special about it.

>> Source/WebCore/accessibility/AccessibilityTableRow.cpp:152
>> +    for (unsigned i = 0; i < rowChildren.size(); i++) {
> 
> are we able to use the better iterators for accessibility children
> 
> for (const auto& cell in rowChildren)

Ok
Comment 7 Nan Wang 2015-10-09 15:24:41 PDT
Created attachment 262797 [details]
patch
Comment 8 chris fleizach 2015-10-09 17:11:59 PDT
Comment on attachment 262797 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityTableCell.cpp:376
> +    const AtomicString& colIndexValue = getAttribute(aria_colindexAttr);

should these methods verify that the table cell has a role="grid cell"
Comment 9 chris fleizach 2015-10-09 17:20:45 PDT
Comment on attachment 262797 [details]
patch

ok
Comment 10 WebKit Commit Bot 2015-10-09 18:09:05 PDT
Comment on attachment 262797 [details]
patch

Clearing flags on attachment: 262797

Committed r190833: <http://trac.webkit.org/changeset/190833>
Comment 11 WebKit Commit Bot 2015-10-09 18:09:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 James Craig 2015-10-12 19:20:57 PDT
Comment on attachment 262797 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityTableCell.cpp:376
>> +    const AtomicString& colIndexValue = getAttribute(aria_colindexAttr);
> 
> should these methods verify that the table cell has a role="grid cell"

I don't think so. These could work with "cell", "gridcell" (when appropriate), or the implicit role from the th/td.
Comment 13 Nan Wang 2015-10-16 13:55:18 PDT
Created attachment 263325 [details]
example html

Added an example html for testing.
Comment 14 James Craig 2016-12-13 23:43:22 PST
*** Bug 163644 has been marked as a duplicate of this bug. ***