[webkit-reviews] review granted: [Bug 111222] AX: aria-label does not override text content on iOS : [Attachment 191046] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 4 15:13:58 PST 2013


David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 111222: AX: aria-label does not override text content on iOS
https://bugs.webkit.org/show_bug.cgi?id=111222

Attachment 191046: patch
https://bugs.webkit.org/attachment.cgi?id=191046&action=review

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191046&action=review


r=me, but consider expanding the layout test to cover all four scenarios if
possible.

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:685
> +    if ([axTitle length] && [axDescription length])
> +	   return [axTitle stringByAppendingFormat:@", %@", axDescription];
> +    else if ([axTitle length])
> +	   return axTitle;
> +    else if ([axDescription length])
> +	   return axDescription;

Nit: Might be worth pulling [axTitle length] and [axDescription length] into
local variables since they're used multiple times here.

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:687
> +    else if ([self stringValueShouldBeUsedInLabel])
> +	   return m_object->stringValue();

Also, is it possible to test all of these code paths (title and description,
title only, description only, neither) using a layout test?

If so, the layout test should be changed to test all four scenarios.


More information about the webkit-reviews mailing list