The AX code has heuristics that try to avoid exposing layout tables as data tables. However, when a section of the document is contentEditable, all tables should be treated as data tables, otherwise users may not be able to work with rich text editors that allow creating and editing tables. Firefox already works this way (if it's contenteditable, all tables are data tables), I suggest WebKit follow that lead to keep things simple, unless there are any good arguments not to.
Created attachment 143428 [details] Patch
Attachment 143428 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/acce..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 143432 [details] Patch
Don't you need to update the expected file in platform/mac/accessibility?
Comment on attachment 143432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143432&action=review looks like we're missing an expected file for the table-detection test > Source/WebCore/accessibility/AccessibilityTable.cpp:99 > + if (m_renderer->node() && m_renderer->node()->isContentEditable()) add a comment explaining why you're doing this please > Source/WebCore/accessibility/AccessibilityTable.cpp:100 > + return true; you should be able to call just node() && node()->isContentEditable() > LayoutTests/accessibility/table-detection.html:105 > + <td nowrap="1" class="labelPol_080915_Lehman_Contributions"> probably unnecessary to include class="labelPol_080915_Lehman_Contributions"
(In reply to comment #5) > probably unnecessary to include class="labelPol_080915_Lehman_Contributions" I think this was so that it could copy a table above. My preference would be to copy something more trivial, like a table with one cell or a table with two cells but no borders - rather than this more complicated example.
Comment on attachment 143432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143432&action=review >> Source/WebCore/accessibility/AccessibilityTable.cpp:99 >> + if (m_renderer->node() && m_renderer->node()->isContentEditable()) > > add a comment explaining why you're doing this please Done. >> Source/WebCore/accessibility/AccessibilityTable.cpp:100 >> + return true; > > you should be able to call just node() && node()->isContentEditable() Done. >> LayoutTests/accessibility/table-detection.html:105 >> + <td nowrap="1" class="labelPol_080915_Lehman_Contributions"> > > probably unnecessary to include class="labelPol_080915_Lehman_Contributions" Done as per Dominic's suggestion.
Created attachment 143481 [details] Patch
Comment on attachment 143481 [details] Patch looks good
Comment on attachment 143481 [details] Patch Clearing flags on attachment: 143481 Committed r118143: <http://trac.webkit.org/changeset/118143>
All reviewed patches have been landed. Closing bug.