AX: VoiceOver can't press on items

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

Now when I visit apple.com with WebKit Nightly I hear "AXScrollToVisible Lunch" when I Control-Option-space on a link

[chris fleizach]

Created attachment 196702 [details]
patch

[chris fleizach]

rdar://13580932

[chris fleizach]

Adding Tim to help with review

[Tim Horton]

Comment on attachment 196702 [details]
patch 

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:938
> -    static NSArray *defaultElementActions = [[NSArray alloc] initWithObjects:NSAccessibilityShowMenuAction, NSAccessibilityScrollToVisibleAction, nil];
> +    static NSArray *defaultElementActions = [[NSArray alloc] initWithObjects:NSAccessibilityShowMenuAction, scrollToVisibleAction, nil];
>  
>      // Action elements allow Press.
>      // The order is important to VoiceOver, which expects the 'default' action to be the first action. In this case the default action should be press.
> -    static NSArray *actionElementActions = [[NSArray alloc] initWithObjects:NSAccessibilityPressAction, NSAccessibilityShowMenuAction, NSAccessibilityScrollToVisibleAction, nil];
> +    static NSArray *actionElementActions = [[NSArray alloc] initWithObjects:NSAccessibilityPressAction, NSAccessibilityShowMenuAction, scrollToVisibleAction, nil];

This is just *asking* for someone to come along and add something after scrollToVisibleAction and wonder why it doesn't work. Can we organize this differently somehow? Even if you split this line and put the ifdefs in the middle of it, that would seem better to me (we do that elsewhere).

[chris fleizach]

Created attachment 196706 [details]
patch

good point. how does this look

[Tim Horton]

Comment on attachment 196706 [details]
patch

That works too.

[WebKit Commit Bot]

Comment on attachment 196706 [details]
patch

Clearing flags on attachment: 196706

Committed r147824: <http://trac.webkit.org/changeset/147824>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tim Horton]

Comment on attachment 196706 [details]
patch

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

> LayoutTests/platform/mac-future/accessibility/press-action-is-first-expected.txt:2
> +This tests that the AXPressAction comes first for activatable items instead of the scroll to visible action. This is needed for screenreaders to operate correctly.

Please don't land test results in mac-future.

[chris fleizach]

(In reply to comment #9)
> (From update of attachment 196706 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196706&action=review
> 
> > LayoutTests/platform/mac-future/accessibility/press-action-is-first-expected.txt:2
> > +This tests that the AXPressAction comes first for activatable items instead of the scroll to visible action. This is needed for screenreaders to operate correctly.
> 
> Please don't land test results in mac-future.

Should this be removed then

[Jer Noble]

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 196706 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=196706&action=review
> > 
> > > LayoutTests/platform/mac-future/accessibility/press-action-is-first-expected.txt:2
> > > +This tests that the AXPressAction comes first for activatable items instead of the scroll to visible action. This is needed for screenreaders to operate correctly.
> > 
> > Please don't land test results in mac-future.
> 
> Should this be removed then

I cleaned things up in r147943. <http://trac.webkit.org/changeset/147943>

[chris fleizach]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (From update of attachment 196706 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=196706&action=review
> > > 
> > > > LayoutTests/platform/mac-future/accessibility/press-action-is-first-expected.txt:2
> > > > +This tests that the AXPressAction comes first for activatable items instead of the scroll to visible action. This is needed for screenreaders to operate correctly.
> > > 
> > > Please don't land test results in mac-future.
> > 
> > Should this be removed then
> 
> I cleaned things up in r147943. <http://trac.webkit.org/changeset/147943>

Thanks