Bug 166571 - AX: Need to expose frames to iOS Accessibility
Summary: AX: Need to expose frames to iOS Accessibility
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-12-29 00:06 PST by chris fleizach
Modified: 2016-12-29 10:47 PST (History)
9 users (show)

See Also:


Attachments
patch (1.73 KB, patch)
2016-12-29 00:11 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (1.51 KB, patch)
2016-12-29 00:13 PST, chris fleizach
darin: review+
Details | Formatted Diff | Diff
patch for landing (1.50 KB, patch)
2016-12-29 09:40 PST, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2016-12-29 00:06:50 PST
To support navigation by frame for VoiceOver on iOS, we need to expose the frame ancestor
Comment 1 Radar WebKit Bug Importer 2016-12-29 00:08:22 PST
<rdar://problem/29823724>
Comment 2 chris fleizach 2016-12-29 00:11:44 PST
Created attachment 297825 [details]
patch
Comment 3 chris fleizach 2016-12-29 00:13:26 PST
Created attachment 297826 [details]
patch
Comment 4 Darin Adler 2016-12-29 01:30:55 PST
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.
Comment 5 chris fleizach 2016-12-29 09:23:36 PST
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.
Comment 6 chris fleizach 2016-12-29 09:40:12 PST
Created attachment 297835 [details]
patch for landing
Comment 7 WebKit Commit Bot 2016-12-29 10:47:22 PST
Comment on attachment 297835 [details]
patch for landing

Clearing flags on attachment: 297835

Committed r210205: <http://trac.webkit.org/changeset/210205>
Comment 8 WebKit Commit Bot 2016-12-29 10:47:25 PST
All reviewed patches have been landed.  Closing bug.