Bug 33088 – MSAA: <select> elements should broadcast value change events

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Jon Honeycutt]

WebKit should broadcast an EVENT_OBJECT_VALUECHANGE event when the selected index of a non-multiple <select> element changes.

<rdar://problem/7332364>

[Jon Honeycutt]

Created attachment 45730 [details]
patch

[Darin Adler]

Comment on attachment 45730 [details]
patch

> +    enum PostType { PostSynchronously, PostAsynchronously };
> +    void postNotification(RenderObject*, AXNotification, bool postToElement, PostType = PostAsynchronously);

One the one hand, it's good to not pollute the global WebCore namespace with something like PostType. But on the other, the whole point of such enums is to make code more readable, and if call sites have to utter AXObjectCache::PostSynchronously it's pretty awkward. I suggest moving such enums out of the class.

r=me as is though

[Jon Honeycutt]

(In reply to comment #2)
> (From update of attachment 45730 [details])
> > +    enum PostType { PostSynchronously, PostAsynchronously };
> > +    void postNotification(RenderObject*, AXNotification, bool postToElement, PostType = PostAsynchronously);
> 
> One the one hand, it's good to not pollute the global WebCore namespace with
> something like PostType. But on the other, the whole point of such enums is to
> make code more readable, and if call sites have to utter
> AXObjectCache::PostSynchronously it's pretty awkward. I suggest moving such
> enums out of the class.
> 
> r=me as is though

I made this change.

Committed in r52773.

[Adam Roben (:aroben)]

Comment on attachment 45730 [details]
patch

> +++ b/WebCore/dom/SelectElement.cpp
> @@ -349,6 +349,8 @@ void SelectElement::setSelectedIndex(SelectElementData& data, Element* element,
>          data.setUserDrivenChange(userDrivenChange);
>          if (fireOnChangeNow)
>              menuListOnChange(data, element);
> +        if (RenderMenuList* menuList = toRenderMenuList(element->renderer()))
> +            menuList->didSetSelectedIndex();
>      }

What if the <select> has no renderer (e.g., it's display:none and someone modifies selectedIndex via JS)? Should we still broadcast in that case?

[Jon Honeycutt]

(In reply to comment #4)
> (From update of attachment 45730 [details])
> > +++ b/WebCore/dom/SelectElement.cpp
> > @@ -349,6 +349,8 @@ void SelectElement::setSelectedIndex(SelectElementData& data, Element* element,
> >          data.setUserDrivenChange(userDrivenChange);
> >          if (fireOnChangeNow)
> >              menuListOnChange(data, element);
> > +        if (RenderMenuList* menuList = toRenderMenuList(element->renderer()))
> > +            menuList->didSetSelectedIndex();
> >      }
> 
> What if the <select> has no renderer (e.g., it's display:none and someone
> modifies selectedIndex via JS)? Should we still broadcast in that case?

No, a <select> with no renderer would not be exposed in our accessibility tree.