Bug 129369

Summary: AX: accessibility data table heuristics fail on this jQuery table
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, dmazzoni, jdiggs, mario, samuel_white, webkit-bug-importer, webkit-ews
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 130120    
Bug Blocks:    
Attachments:
Description Flags
patch mario: review+

Description James Craig 2014-02-26 01:42:02 PST
AX: accessibility data table heuristics fail on this jQuery table. Not yet sure why.

http://view.jquerymobile.com/1.3.2/dist/demos/widgets/table-reflow/
Comment 1 Radar WebKit Bug Importer 2014-02-26 01:42:11 PST
<rdar://problem/16171130>
Comment 2 James Craig 2014-02-26 04:13:06 PST
It appears it's because the table element's display property is set to table-row-group instead of table.
Comment 3 chris fleizach 2014-03-12 23:24:31 PDT
Created attachment 226577 [details]
patch
Comment 4 James Craig 2014-03-13 01:24:15 PDT
Comment on attachment 226577 [details]
patch

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

> LayoutTests/accessibility/table-detection.html:1
> -<html>
> +<!DOCTYPE html>

You need both the DTD declartion and the opening <html> tag.

<!DOCTYPE html>
<html>
Comment 5 Mario Sanchez Prada 2014-03-13 05:33:57 PDT
Comment on attachment 226577 [details]
patch

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

This new heuristic makes sense to me. Just please consider the nits below + James's comment before landing

> Source/WebCore/ChangeLog:14
> +        (WebCore::AccessibilityRenderObject::activeDescendant):

The changes here seem to be unrelated to the patch, but I'm ok anyway as I don't think filing a new bug for those is worth it.

> Source/WebCore/accessibility/AccessibilityTable.cpp:140
> +        // if there is a caption element, summary, THEAD, or TFOOT section, it's most certainly a data table.

"if"->"If" (yes, I know it's just re-indented code :-))

> Source/WebCore/accessibility/AccessibilityTable.cpp:144
> +        // if someone used "rules" attribute than the table should appear

"if"->"If" / "than"->"then" / missing period at the end

> Source/WebCore/accessibility/AccessibilityTable.cpp:148
> +        // if there's a colgroup or col element, it's probably a data table.

"if"->"If"
Comment 6 chris fleizach 2014-03-13 08:57:21 PDT
(In reply to comment #5)
> (From update of attachment 226577 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226577&action=review
> 
> This new heuristic makes sense to me. Just please consider the nits below + James's comment before landing
> 
> > Source/WebCore/ChangeLog:14
> > +        (WebCore::AccessibilityRenderObject::activeDescendant):
> 
> The changes here seem to be unrelated to the patch, but I'm ok anyway as I don't think filing a new bug for those is worth it.
> 

I meant to add a comment in the ChangeLog. This change was necessary otherwise getting the active descendant would crash in this kind of table (once it was detected as a table)

> > Source/WebCore/accessibility/AccessibilityTable.cpp:140
> > +        // if there is a caption element, summary, THEAD, or TFOOT section, it's most certainly a data table.
> 
> "if"->"If" (yes, I know it's just re-indented code :-))
> 
> > Source/WebCore/accessibility/AccessibilityTable.cpp:144
> > +        // if someone used "rules" attribute than the table should appear
> 
> "if"->"If" / "than"->"then" / missing period at the end
> 
> > Source/WebCore/accessibility/AccessibilityTable.cpp:148
> > +        // if there's a colgroup or col element, it's probably a data table.
> 
> "if"->"If"

Thanks!
Comment 7 chris fleizach 2014-03-13 09:29:54 PDT
http://trac.webkit.org/changeset/165535