[Webkit-unassigned] [Bug 122015] [ATK] Expose aria-invalid as a text attribute (not object attribute)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122015





--- Comment #3 from chris fleizach <cfleizach at apple.com>  2013-09-27 09:44:21 PST ---
(From update of attachment 212816)
View in context: https://bugs.webkit.org/attachment.cgi?id=212816&action=review

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:308
> +        String invalidValue = invalidStatus == "spelling" || invalidStatus == "grammar" ? invalidStatus : "true";

i think invalid could be anything, so i think you want to just use the actual value instead of casting other types to true

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:69
> +static String atkAttributeValueToCoreAttributeValue(AtkAttributeType type, String& id, String& value)

these params look like they should be const

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:75
> +        if (id == "sort" && !value.isEmpty()) {

Maybe you should have at the top of some file a list of these hard coded strings like

const String atkSortAttributeName = "sort";

so that you're not referencing literals

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:86
> +    if (type == AtkAttributeTypeText) {

this should be an else if, since the first if does not always return

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:93
> +    // Return the passed value unchanged if not filtered.

this comment seems unnecessary unless you say why you are passing it unfiltered

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:90
> +    if (type == AtkAttributeTypeText) {

ditto about else if

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.