Bug 91911 – WebKit should expose @title as label (AXTitle or AXDescription) sometimes instead of AXHelp, according to the ARIA text alt computation

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

[chris fleizach]

WebKit should expose @title as label (AXTitle or AXDescription) sometimes instead of AXHelp, according to the ARIA text alt computation

http://www.w3.org/WAI/PF/aria/complete#textalternativecomputation

Logic should be:
if AXDescription && AXTitle == empty, then 
	use title attribute as AXDescription
	do not return title attribute as AXHelp
For AXHelp
	- use aria-help first, then use title attribute, unless being used for description

[chris fleizach]

Created attachment 153629 [details]
patch

[chris fleizach]

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

[Darin Adler]

Comment on attachment 153629 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1032
> +    String descriptiveText = accessibilityDescription();

If the function is named description, then I suggest naming the local variable description too. If they should be named descriptive text, then both should use that phrase instead of description.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1039
> +            // The title attribute should be used as help text, unless it is already being used as descriptive text.

I think you should omit the comma.

This comment says what the code does, but not why.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1501
> +    // The description should fall back to the title attribute as a last resort.
> +    if (title().isEmpty())
> +        return getAttribute(titleAttr);
> +    
>      return String();

I don’t understand the if statement. If title() returns a non-empty string, we return a null accessibility description. Why is that what we want? Probably needs a why comment.

[chris fleizach]

(In reply to comment #3)
> (From update of attachment 153629 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153629&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1032
> > +    String descriptiveText = accessibilityDescription();
> 
> If the function is named description, then I suggest naming the local variable description too. If they should be named descriptive text, then both should use that phrase instead of description.
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1039
> > +            // The title attribute should be used as help text, unless it is already being used as descriptive text.
> 
> I think you should omit the comma.
> 
> This comment says what the code does, but not why.
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1501
> > +    // The description should fall back to the title attribute as a last resort.
> > +    if (title().isEmpty())
> > +        return getAttribute(titleAttr);
> > +    
> >      return String();
> 
> I don’t understand the if statement. If title() returns a non-empty string, we return a null accessibility description. Why is that what we want? Probably needs a why comment.

Thanks for the feedback. I will make all these changes. As to your final question, the comment I will add to explain it is the following: 

+    // An elements descriptive text is comprised of title() (what's visible on the screen) and accessibilityDescription() (other descriptive text).
+    // Both are used to generate what a screen reader speaks.
+    // If this point is reached (i.e. there's no accessibilityDescription) and there's no title(), we should fallback to using the title attribute.
+    // The title attribute is normally used as help text (because it is a tooltip), but if there is nothing else available, this should be used (according to ARIA).

[chris fleizach]

http://trac.webkit.org/changeset/123884
in response to Darin's comments