Summary: | [ATK] Wrong selected element at a given index in a list box. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Krzysztof Czech <k.czech> | ||||||||||
Component: | Accessibility | Assignee: | Krzysztof Czech <k.czech> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dbates, dmazzoni, jcraig, jdiggs, mario, samuel_white, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 129970 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Krzysztof Czech
2014-02-19 05:16:07 PST
Created attachment 224626 [details]
patch
Created attachment 224628 [details]
patch
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 Created attachment 224742 [details]
patch
> > 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. Created attachment 224752 [details]
patch - fixed win build
Comment on attachment 224752 [details] patch - fixed win build Clearing flags on attachment: 224752 Committed r164577: <http://trac.webkit.org/changeset/164577> All reviewed patches have been landed. Closing bug. (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>. (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 |