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: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
chris fleizach
2012-07-20 17:16:41 PDT
Created attachment 153629 [details]
patch
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. (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). http://trac.webkit.org/changeset/123884 in response to Darin's comments |