Bug 129039 – [ATK] Wrong selected element at a given index in a list box.

• Component: Accessibility
• Status: RESOLVED FIXED

[Krzysztof Czech]

The selected element at a given index was wrong. Current implementation tried to get one from collection of all selected elements instead of collection of real children of a listbox.

[Radar WebKit Bug Importer]

<rdar://problem/16108701>

[Krzysztof Czech]

Created attachment 224626 [details]
patch

[Krzysztof Czech]

Created attachment 224628 [details]
patch

[chris fleizach]

Comment on attachment 224628 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224628&action=review

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceSelection.cpp:113
> +            return 0;

return nullptr

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:1224
> +        indexNumber = JSValueToNumber(context, arguments[0], exception);

you might want to return early after you set indexNumber if it still equals -1, because once you cast to unsigned in removeSelectionChild, it will make it a UINT_MAX

> Tools/DumpRenderTree/AccessibilityUIElement.h:152
> +    void removeSelectionChildAtIndex(unsigned) const;

this should probably just be removeSelectionAtIndex. I feel like the "selectionChild" sounds strange in a "remove" context

[Krzysztof Czech]

Created attachment 224742 [details]
patch

[Krzysztof Czech]

> > Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceSelection.cpp:113
> > +            return 0;
> 
> return nullptr
Done, corrected other places as well.
> 
> > Tools/DumpRenderTree/AccessibilityUIElement.cpp:1224
> > +        indexNumber = JSValueToNumber(context, arguments[0], exception);
> 
> you might want to return early after you set indexNumber if it still equals -1, because once you cast to unsigned in removeSelectionChild, it will make it a UINT_MAX
Done.
> 
> > Tools/DumpRenderTree/AccessibilityUIElement.h:152
> > +    void removeSelectionChildAtIndex(unsigned) const;
> 
> this should probably just be removeSelectionAtIndex. I feel like the "selectionChild" sounds strange in a "remove" context
Done.

Thanks.

[Krzysztof Czech]

Created attachment 224752 [details]
patch - fixed win build

[WebKit Commit Bot]

Comment on attachment 224752 [details]
patch - fixed win build

Clearing flags on attachment: 224752

Committed r164577: <http://trac.webkit.org/changeset/164577>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

(In reply to comment #8)
> (From update of attachment 224752 [details])
> Clearing flags on attachment: 224752
> 
> Committed r164577: <http://trac.webkit.org/changeset/164577>

This broke the iOS DumpRenderTree build because AccessibilityUIElementMac.mm is excluded when building DumpRenderTree and neither AccessibilityUIElement::selectedChildAtIndex() nor AccessibilityUIElement::selectedChildrenCount() are defined in AccessibilityUIElementIOS.mm.

Build fix committed in <http://trac.webkit.org/changeset/164595>.

[Joanmarie Diggs]

(In reply to comment #0)
> The selected element at a given index was wrong. Current implementation
> tried to get one from collection of all selected elements instead of
> collection of real children of a listbox.

A git blame led me here.

According to the documentation [1] atk_selection_ref_selection() takes "a gint specifying the index in the selection set. (e.g. the ith selection as opposed to the ith child)."

[1] https://developer.gnome.org/atk/stable/AtkSelection.html#atk-selection-ref-selection