[webkit-reviews] review granted: [Bug 171178] AX: Treat cells with ARIA table cell properties as cells : [Attachment 308909] Patch

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has granted Joanmarie Diggs (irc: joanie)
<jdiggs at igalia.com>'s request for review:
Bug 171178: AX: Treat cells with ARIA table cell properties as cells
https://bugs.webkit.org/show_bug.cgi?id=171178

Attachment 308909: Patch

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




--- Comment #5 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 308909
  --> https://bugs.webkit.org/attachment.cgi?id=308909
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