Bug 91911

Summary: WebKit should expose @title as label (AXTitle or AXDescription) sometimes instead of AXHelp, according to the ARIA text alt computation
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch andersca: review+

Description chris fleizach 2012-07-20 17:16:41 PDT
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
Comment 1 chris fleizach 2012-07-20 17:18:02 PDT
Created attachment 153629 [details]
patch
Comment 2 chris fleizach 2012-07-26 10:24:10 PDT
http://trac.webkit.org/changeset/123767
Comment 3 Darin Adler 2012-07-26 16:18:51 PDT
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.
Comment 4 chris fleizach 2012-07-26 17:11:04 PDT
(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).
Comment 5 chris fleizach 2012-07-27 10:39:43 PDT
http://trac.webkit.org/changeset/123884
in response to Darin's comments