[Webkit-unassigned] [Bug 121413] [ATK] Extends atk value interface to return proper checkbox states.

[bugzilla-daemon at webkit.org]

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


Mario Sanchez Prada <mario at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #211747|review?                     |review+
               Flag|                            |




--- Comment #5 from Mario Sanchez Prada <mario at webkit.org>  2013-09-16 03:34:19 PST ---
(From update of attachment 211747)
View in context: https://bugs.webkit.org/attachment.cgi?id=211747&action=review

The patch looks good already, yet there are some minor changes that I think we should do before landing

> LayoutTests/ChangeLog:8
> +        Sharing mac tests with other ports (GTK/EFL).

Great!

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceValue.cpp:39
> +static float webkitAccessibleValueGetCurrentValue(AccessibilityObject* coreObject)

I would probably call this webkitAccessibleValueValueForAccessibilityObject(), to make it clearer from the caller (and to make it more explicit in the ChangeLog that you added a new function here)

> Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:1038
> +    if (role == SliderRole || role == SpinButtonRole || role == ScrollBarRole || role == ProgressIndicatorRole || role == CheckBoxRole)

I think we should replace this condition with (coreObject->supportsRangeValue() || coreObject->isCheckboxOrRadio())

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