Summary: | AX: ARIA 1.1 implement aria-colcount, aria-colindex, aria-colspan, aria-rowcount, aria-rowindex and aria-rowspan | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nan Wang <n_wang> | ||||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, esprehn+autocc, gyuyoung.kim, jcraig, jdiggs, mario, n_wang, samuel_white, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Nan Wang
2015-09-08 10:50:13 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.
Created attachment 262789 [details]
patch
Fixed iOS build failure.
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 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 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 Created attachment 262797 [details]
patch
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 on attachment 262797 [details]
patch
ok
Comment on attachment 262797 [details] patch Clearing flags on attachment: 262797 Committed r190833: <http://trac.webkit.org/changeset/190833> All reviewed patches have been landed. Closing bug. 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. Created attachment 263325 [details]
example html
Added an example html for testing.
*** Bug 163644 has been marked as a duplicate of this bug. *** |