AXSliders are missing required attributes and actions

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

AXSliders are missing required attributes and actions.

following attribute:
AXOrientation

and the following actions:
AXIncrement
AXDecrement

[chris fleizach]

Created attachment 34079 [details]
patch

[Darin Adler]

Comment on attachment 34079 [details]
patch

> +    // There's no support in ARIA to specify horizontal or vertical, so everything should be horizontal.
> +    if ((m_object->isSlider() || m_object->isProgressIndicator()) && [attributeName isEqualToString:NSAccessibilityOrientationAttribute])
> +        return NSAccessibilityHorizontalOrientationValue;

I'm not sure that's right. For <input type=range> we do know for sure whether it's horizontal or vertical, so I'm not sure why ARIA makes this less practical for that. And in the ARIA case is there a chance we could use the shape of the rendered object to guess about horizontal vs. vertical?

> +    JSStringRef action = NULL;

We use 0 for this in WebKit coding style, not NULL.

r=me

[chris fleizach]

i'm going to re-submit the patch since the changes to calculate orientation or a little involved

[chris fleizach]

Comment on attachment 34079 [details]
patch

based on darin's feedback, i added a portion of new code to accurately return the orientation

[chris fleizach]

Created attachment 34086 [details]
patch

[Darin Adler]

Comment on attachment 34086 [details]
patch

> +    if (bounds.size().width() > bounds.size().height())
> +        return AccessibilityOrientationHorizontal;
> +    if (bounds.size().height() > bounds.size().width())
> +        return AccessibilityOrientationVertical;
> +    return AccessibilityOrientationUnknown;

I think it would be OK to break the tie here and use horizontal. I don't think we need a separate unknown concept since this is only a guess anyway. But if you think there's a strong-enough benefit to distinguishing unknown, then that's fine with me.

Not having unknown would also reduce the need to have a new type for orientation. We could share ScrollbarOrientation -- maybe we'd have to rename it though.

> +    ControlPart styleAppearance = style->appearance();
> +    if (styleAppearance == SliderHorizontalPart || styleAppearance == MediaSliderPart)
> +        return AccessibilityOrientationHorizontal;
> +    if (styleAppearance == SliderVerticalPart)
> +        return AccessibilityOrientationVertical;
> +    
> +    return AccessibilityOrientationUnknown;

I normally suggest using switch statements for something like this since if you don't include a default then you get a warning about any parts not listed. That way if someone adds a new part, they have to look at this code and it makes it more likely someone adding a new type of slider will look here.

> +    if ([attributeName isEqualToString:NSAccessibilityOrientationAttribute]) {
> +        AccessibilityOrientation elementOrientation = m_object->orientation();
> +        if (elementOrientation == AccessibilityOrientationVertical)
> +            return NSAccessibilityVerticalOrientationValue;
> +        if (elementOrientation == AccessibilityOrientationHorizontal)
> +            return NSAccessibilityHorizontalOrientationValue;
> +        return nil;
> +    }

Is it really OK to return nil?

> +enum AccessibilityOrientation {
> +    AccessibilityOrientationUnknown = 0,
> +    AccessibilityOrientationVertical,
> +    AccessibilityOrientationHorizontal,
> +};

The explicit "= 0" is not needed or helpful here.

r=me

[chris fleizach]

Its ok to return nil for orientation on the mac, if not known

i'd like to keep the AccessibilityOrientation, because on the Mac they also have a Circular orientation. (for a circular slider). its possible that might come to webkit in some form

We can get rid of unknown and default to horizontal

[chris fleizach]

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