[Webkit-unassigned] [Bug 25679] [GTK] Improve accessibility of focusable lists

[bugzilla-daemon at webkit.org]

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


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41817|review?                     |review-
               Flag|                            |




--- Comment #8 from Xan Lopez <xan.lopez at gmail.com>  2009-10-26 00:35:31 PDT ---
(From update of attachment 41817)
>  
>  static void setAtkStateSetFromCoreObject(AccessibilityObject* coreObject, AtkStateSet* stateSet)
>  {
> -    // Please keep the state list in alphabetical order
> +    AccessibilityObject* parent = coreObject->parentObject();
> +    bool isListBoxOption = parent && parent->isListBox();
> +    if (isListBoxOption)
> +        coreObject = static_cast<AccessibilityListBoxOption*>(coreObject);

I don't understand what this cast is for?

>  
> +    // Please keep the state list in alphabetical order
>      if (coreObject->isChecked())
>          atk_state_set_add_state(stateSet, ATK_STATE_CHECKED);
>  
>      // FIXME: isReadOnly does not seem to do the right thing for
>      // controls, so check explicitly for them
> -    if (!coreObject->isReadOnly() ||
> -        (coreObject->isControl() && coreObject->canSetValueAttribute()))
> +    if ((!coreObject->isReadOnly() ||
> +        (coreObject->isControl() && coreObject->canSetValueAttribute())) &&
> +        !isListBoxOption)
>          atk_state_set_add_state(stateSet, ATK_STATE_EDITABLE);

I'd go ahead and add a comment explaining that we don't want this in ATK for
whatever reason, otherwise the if is impossible to understand.
>  
>      // FIXME: Put both ENABLED and SENSITIVE together here for now
> @@ -408,8 +414,23 @@ static void setAtkStateSetFromCoreObject(AccessibilityObject* coreObject, AtkSta
>  
>      // TODO: ATK_STATE_SELECTABLE_TEXT
>  
> -    if (coreObject->isSelected())
> +    if (coreObject->canSetSelectedAttribute()) {
> +        atk_state_set_add_state(stateSet, ATK_STATE_SELECTABLE);
> +        // Items in focusable lists in Gtk have both STATE_SELECT{ABLE,ED}
> +        // and STATE_FOCUS{ABLE,ED}. We'll fake the latter based on the
> +        // former.
> +        if (isListBoxOption)
> +            atk_state_set_add_state(stateSet, ATK_STATE_FOCUSABLE);
> +    }
> +
> +    if (coreObject->isSelected()) {
>          atk_state_set_add_state(stateSet, ATK_STATE_SELECTED);
> +        // Items in focusable lists in Gtk have both STATE_SELECT{ABLE,ED}
> +        // and STATE_FOCUS{ABLE,ED}. We'll fake the latter based on the
> +        // former.
> +        if (isListBoxOption)
> +            atk_state_set_add_state(stateSet, ATK_STATE_FOCUSED);
> +    }

Good!

Marking r- to fix those two details.

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