Bug 131208

Summary: Regression: AX: image labels no longer exposed to AX API in SVG test case
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jdiggs, mario, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.webkit.org/blog-files/aria1.0/africa_large.svg
Attachments:
Description Flags
patch dbates: review+

Description James Craig 2014-04-03 20:08:38 PDT
Regression: AX: image labels no longer exposed to AX API in SVG test case. I believe this is a recent regression.

http://www.webkit.org/blog-files/aria1.0/africa_large.svg
Comment 1 Radar WebKit Bug Importer 2014-04-03 20:09:00 PDT
<rdar://problem/16520975>
Comment 2 chris fleizach 2014-04-07 22:35:10 PDT
Created attachment 228805 [details]
patch
Comment 3 Daniel Bates 2014-04-07 23:20:41 PDT
Comment on attachment 228805 [details]
patch

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

This patch looks straightforward to me.

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1849
>  static String accessibleNameForNode(Node* node)

Nit: We should change the parameter type from Node* to Node& since this function assumes node is a non-null pointer. Otherwise, we should check/assert that node is non-null.

> LayoutTests/accessibility/svg-labelledby.html:6
> +<body id="body">

The HTML attribute id is unnecessary as we never reference this element by id in this document. Moreover, it's sufficient it use document.body to obtain a reference to the HTML Body element instead of assigning an id to it.

> LayoutTests/accessibility/svg-labelledby.html:13
> +  </path>

Nit: This closing tag has a different amount of indentation that its opening tag (line 11).
Comment 4 chris fleizach 2014-04-07 23:31:41 PDT
(In reply to comment #3)
> (From update of attachment 228805 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228805&action=review
> 
> This patch looks straightforward to me.
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1849
> >  static String accessibleNameForNode(Node* node)
> 
> Nit: We should change the parameter type from Node* to Node& since this function assumes node is a non-null pointer. Otherwise, we should check/assert that node is non-null.
> 
> > LayoutTests/accessibility/svg-labelledby.html:6
> > +<body id="body">
> 
> The HTML attribute id is unnecessary as we never reference this element by id in this document. Moreover, it's sufficient it use document.body to obtain a reference to the HTML Body element instead of assigning an id to it.
> 
> > LayoutTests/accessibility/svg-labelledby.html:13
> > +  </path>
> 
> Nit: This closing tag has a different amount of indentation that its opening tag (line 11).

Thanks, I'll take care of all of these
Comment 5 chris fleizach 2014-04-08 00:59:58 PDT
http://trac.webkit.org/changeset/166917