[webkit-reviews] review granted: [Bug 170475] AX: On generic container elements, WebKit should distinguish between tooltip (e.g. @title) and label (e.g. @aria-label) attributes : [Attachment 316433] patch

[bugzilla-daemon at webkit.org]

Joanmarie Diggs (irc: joanie) <jdiggs at igalia.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 170475: AX: On generic container elements, WebKit should distinguish
between tooltip (e.g. @title) and label (e.g. @aria-label) attributes
https://bugs.webkit.org/show_bug.cgi?id=170475

Attachment 316433: patch

https://bugs.webkit.org/attachment.cgi?id=316433&action=review




--- Comment #12 from Joanmarie Diggs (irc: joanie) <jdiggs at igalia.com> ---
Comment on attachment 316433
  --> https://bugs.webkit.org/attachment.cgi?id=316433
patch

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

r+ BUT it would be great if you could tweak the test to use the
shared-platform-friendlier helper methods mentioned inline. Thanks in advance!

>
LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text.html:2
3
> +	    
shouldBe("accessibilityController.accessibleElementById('div1').helpText",
"'AXHelp: test1'");

Your platform's "AXHelp" is my platform's "description" is the W3C AccName
spec's "description". A while back, I added some methods to
resources/accessibility-helper.js to address this very issue. For the above
test case, platformValueForW3CDescription() should do it:
https://trac.webkit.org/browser/trunk/LayoutTests/resources/accessibility-helpe
r.js#L53. If you want to keep the shouldBe() and have a single, shared
expectations file, then the default value for the includeSource argument is
what you want, though it will cause the "AXHelp: " prefix to be dropped. If you
want to preserve the "AXHelp: " prefix for your platform, setting includeSource
to true will accomplish that, though shouldBe() would need to be replaced with
debug() so platform-specific expectations can be created by gardeners.

>
LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text.html:2
4
> +	    
shouldBe("accessibilityController.accessibleElementById('div1').description",
"'AXDescription: '");

Ditto, but platformValueForW3CName().
https://trac.webkit.org/browser/trunk/LayoutTests/resources/accessibility-helpe
r.js#L38