[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 #4 from Mario Sanchez Prada <mario at webkit.org>  2013-09-27 10:03:27 PST ---
(In reply to comment #3)
> (From update of attachment 212816 [details])
> 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

Yeah, I thought of that, but the WAI-ARIA spec explicitly says in [1] to do it the other way:

"For future expansion, the aria-invalid attribute is an enumerated type. Any value not recognized in the list of allowed values MUST be treated by user agents as if the value true had been provided. If the attribute is not present, or its value is false, or its value is an empty string, the default value of false applies."

So my understanding is that we only expose 'grammar' or 'spelling' if those values are set, and 'true' otherwise.

[1] http://www.w3.org/TR/2011/CR-wai-aria-20110118/states_and_properties#aria-invalid

> > 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

That makes sense. I'll change it.

> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:86
> > +    if (type == AtkAttributeTypeText) {
> 
> this should be an else if, since the first if does not always return

Ok.

> > 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

Ok.

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

Ok.

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