Bug 166571 – AX: Need to expose frames to iOS Accessibility

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

[chris fleizach]

To support navigation by frame for VoiceOver on iOS, we need to expose the frame ancestor

[Radar WebKit Bug Importer]

<rdar://problem/29823724>

[chris fleizach]

Created attachment 297825 [details]
patch

[chris fleizach]

Created attachment 297826 [details]
patch

[Darin Adler]

Comment on attachment 297826 [details]
patch

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

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:592
> +    if (const AccessibilityObject* parent = AccessibilityObject::matchedParent(*m_object, false, [] (const AccessibilityObject& object) {
> +        return object.isWebArea();
> +    }))
> +        return parent->wrapper();
> +    return nil;

In WebKit we normally use early exit for errors rather than wrapping success in an if. I also think auto would be good to use here. So I would write:

    auto* parent = AccessibilityObject::matchedParent(*m_object, false, [] (const AccessibilityObject& object) {
        return object.isWebArea();
    });
    if (!parent)
        return nil;
    return parent->wrapper;

But I still think the auto* is an improvement even if you want ot keep it nested the way you have it.

[chris fleizach]

Thanks, will make those changes

(In reply to comment #4)
> Comment on attachment 297826 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=297826&action=review
> 
> > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:592
> > +    if (const AccessibilityObject* parent = AccessibilityObject::matchedParent(*m_object, false, [] (const AccessibilityObject& object) {
> > +        return object.isWebArea();
> > +    }))
> > +        return parent->wrapper();
> > +    return nil;
> 
> In WebKit we normally use early exit for errors rather than wrapping success
> in an if. I also think auto would be good to use here. So I would write:
> 
>     auto* parent = AccessibilityObject::matchedParent(*m_object, false, []
> (const AccessibilityObject& object) {
>         return object.isWebArea();
>     });
>     if (!parent)
>         return nil;
>     return parent->wrapper;
> 
> But I still think the auto* is an improvement even if you want ot keep it
> nested the way you have it.

[chris fleizach]

Created attachment 297835 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 297835 [details]
patch for landing

Clearing flags on attachment: 297835

Committed r210205: <http://trac.webkit.org/changeset/210205>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.