Bug 131208 - Regression: AX: image labels no longer exposed to AX API in SVG test case
Summary: Regression: AX: image labels no longer exposed to AX API in SVG test case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL: http://www.webkit.org/blog-files/aria...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-03 20:08 PDT by James Craig
Modified: 2014-04-08 00:59 PDT (History)
9 users (show)

See Also:


Attachments
patch (4.72 KB, patch)
2014-04-07 22:35 PDT, chris fleizach
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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