Bug 113817 – AX: Bounding paths should be made available through accessibility

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

See

http://examples.simplyaccessible.com/svg-maps/

The <g> elements in the SVG example are not in the AX hierarchy and the <title> field is not being exposed

[chris fleizach]

rdar://13468267

[chris fleizach]

Created attachment 196708 [details]
patch

[Early Warning System Bot]

Comment on attachment 196708 [details]
patch

Attachment 196708 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17526199

[Early Warning System Bot]

Comment on attachment 196708 [details]
patch

Attachment 196708 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17524250

[chris fleizach]

Created attachment 196710 [details]
patch

[Build Bot]

Comment on attachment 196710 [details]
patch

Attachment 196710 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17528276

New failing tests:
platform/mac/accessibility/element-paths.html

[Build Bot]

Created attachment 196714 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Build Bot]

Comment on attachment 196710 [details]
patch

Attachment 196710 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17529333

New failing tests:
platform/mac/accessibility/element-paths.html

[Build Bot]

Created attachment 196717 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.2

[chris fleizach]

Created attachment 196718 [details]
patch

[David Kilzer (:ddkilzer)]

Comment on attachment 196718 [details]
patch

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

r=me, but please investigate why there is a "Move to point" after "Close" on the mac/accessibility/element-paths-expected.txt test results, and future-proof the #if in AccessibilityUIElement.cpp

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:1094
> +#if !PLATFORM(MAC)
> +JSStringRef AccessibilityUIElement::pathDescription() const { return 0; }
> +#endif

For future-proofing, this should be:

#if !PLATFORM(IOS) && !PLATFORM(MAC)

> LayoutTests/platform/mac/accessibility/element-paths-expected.txt:14
> +PASS svg.isAttributeSupported('AXPath') is true
> +SVG path description
> +Start Path
> +	Move to point
> +	Line to
> +	Line to
> +	Close
> +	Move to point

Would it be better to print out the point and line data here?  Or will that change from Mac-to-Mac?

Why is there a "Move to point" after "Close" here and on the two tests below?

> LayoutTests/platform/iphone-simulator/accessibility/element-paths-expected.txt:12
> +SVG path description
> +Start Path
> +	Move to point
> +	Line to
> +	Line to
> +	Close

Would it be best to have specific point and line information here?

[chris fleizach]

(In reply to comment #11)
> (From update of attachment 196718 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196718&action=review
> 
> r=me, but please investigate why there is a "Move to point" after "Close" on the mac/accessibility/element-paths-expected.txt test results, and future-proof the #if in AccessibilityUIElement.cpp
> 
> > Tools/DumpRenderTree/AccessibilityUIElement.cpp:1094
> > +#if !PLATFORM(MAC)
> > +JSStringRef AccessibilityUIElement::pathDescription() const { return 0; }
> > +#endif
> 
> For future-proofing, this should be:
> 
> #if !PLATFORM(IOS) && !PLATFORM(MAC)
> 
> > LayoutTests/platform/mac/accessibility/element-paths-expected.txt:14
> > +PASS svg.isAttributeSupported('AXPath') is true
> > +SVG path description
> > +Start Path
> > +	Move to point
> > +	Line to
> > +	Line to
> > +	Close
> > +	Move to point
> 
> Would it be better to print out the point and line data here?  Or will that change from Mac-to-Mac?
> 

I had that in my first go through but the EWS buildbot failed with the values I had uploaded, so it seems like it might not work out

> Why is there a "Move to point" after "Close" here and on the two tests below?
> 

I don't know. i'll look into that

> > LayoutTests/platform/iphone-simulator/accessibility/element-paths-expected.txt:12
> > +SVG path description
> > +Start Path
> > +	Move to point
> > +	Line to
> > +	Line to
> > +	Close
> 
> Would it be best to have specific point and line information here?

It's possible we can do this for the simulator. I had the same results every time I ran it, but i'm not sure if we'd get different answers on different machines

[chris fleizach]

(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 196718 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=196718&action=review
> > 
> > r=me, but please investigate why there is a "Move to point" after "Close" on the mac/accessibility/element-paths-expected.txt test results, and future-proof the #if in AccessibilityUIElement.cpp
> > 
> > > Tools/DumpRenderTree/AccessibilityUIElement.cpp:1094
> > > +#if !PLATFORM(MAC)
> > > +JSStringRef AccessibilityUIElement::pathDescription() const { return 0; }
> > > +#endif
> > 
> > For future-proofing, this should be:
> > 
> > #if !PLATFORM(IOS) && !PLATFORM(MAC)
> > 
> > > LayoutTests/platform/mac/accessibility/element-paths-expected.txt:14
> > > +PASS svg.isAttributeSupported('AXPath') is true
> > > +SVG path description
> > > +Start Path
> > > +	Move to point
> > > +	Line to
> > > +	Line to
> > > +	Close
> > > +	Move to point
> > 
> > Would it be better to print out the point and line data here?  Or will that change from Mac-to-Mac?
> > 
> 
> I had that in my first go through but the EWS buildbot failed with the values I had uploaded, so it seems like it might not work out
> 
> > Why is there a "Move to point" after "Close" here and on the two tests below?
> > 

I looked into it. It looks like NSBezierPath appends another move to point (all the time) when you do a close subpath

> 
> I don't know. i'll look into that
> 
> > > LayoutTests/platform/iphone-simulator/accessibility/element-paths-expected.txt:12
> > > +SVG path description
> > > +Start Path
> > > +	Move to point
> > > +	Line to
> > > +	Line to
> > > +	Close
> > 
> > Would it be best to have specific point and line information here?
> 
> It's possible we can do this for the simulator. I had the same results every time I ran it, but i'm not sure if we'd get different answers on different machines

[chris fleizach]

http://trac.webkit.org/changeset/148033

[Tim Horton]

Comment on attachment 196718 [details]
patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1503
> +        IntPoint intPoint = (IntPoint)point;

This isn't the right way to cast to IntPoint, please use the constructor (and, you broke the build :D)

[chris fleizach]

(In reply to comment #15)
> (From update of attachment 196718 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196718&action=review
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1503
> > +        IntPoint intPoint = (IntPoint)point;
> 
> This isn't the right way to cast to IntPoint, please use the constructor 

Will fix.

> (and, you broke the build :D)

Which platform is broken? When i look at http://build.webkit.org/waterfall 
I don't see any failures

[chris fleizach]

(In reply to comment #16)
> (In reply to comment #15)
> > (From update of attachment 196718 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=196718&action=review
> > 
> > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1503
> > > +        IntPoint intPoint = (IntPoint)point;
> > 
> > This isn't the right way to cast to IntPoint, please use the constructor 
> 
> Will fix.

Addressed this feedback in

http://trac.webkit.org/changeset/148039

> 
> > (and, you broke the build :D)
> 
> Which platform is broken? When i look at http://build.webkit.org/waterfall 
> I don't see any failures

[Tim Horton]

(In reply to comment #17)
> > 
> > > (and, you broke the build :D)
> > 
> > Which platform is broken? When i look at http://build.webkit.org/waterfall 
> > I don't see any failures

Sent you an email.

[chris fleizach]

(In reply to comment #18)
> (In reply to comment #17)
> > > 
> > > > (and, you broke the build :D)
> > > 
> > > Which platform is broken? When i look at http://build.webkit.org/waterfall 
> > > I don't see any failures
> 
> Sent you an email.

Addressed in
http://trac.webkit.org/changeset/148042

[Raphael Kubo da Costa (:rakuco)]

You also broke WebKitTestRunner for all non-Mac platforms :(

Fixed in <http://trac.webkit.org/changeset/148086>.