Bug 138241 - AX:setSelectedRows loop is incorrect -- ignores the count
Summary: AX:setSelectedRows loop is incorrect -- ignores the count
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Matthew Daiter
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-30 23:24 PDT by chris fleizach
Modified: 2016-09-05 01:22 PDT (History)
12 users (show)

See Also:

Attachments
Patch (1.53 KB, patch)
2015-07-31 14:59 PDT, Matthew Daiter 		cfleizach: review-
cfleizach: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2014-10-30 23:24:16 PDT 
From:
https://bugs.webkit.org/show_bug.cgi?id=126915#c12

> >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1650
> >> +    for (const auto& selectedRow : selectedRows)
> > 
> > I don't believe this change is correct. It ignores the count variable that is set above depending on isMultiSelectable() so I am assuming we can now select more than 1 row even if it is not multiSelectable.
> 
> I think you're right too

This was reported by clang static analyzer.
Would you mind making the fix? I don't feel like writing the layout test for this :)
Comment 1 Radar WebKit Bug Importer 2014-10-30 23:24:24 PDT 
<rdar://problem/18834530>
Comment 2 Radar WebKit Bug Importer 2014-10-30 23:25:14 PDT 
<rdar://problem/18834534>
Comment 3 Matthew Daiter 2015-07-31 14:59:00 PDT 
Created attachment 257963 [details]
Patch
Comment 4 Matthew Daiter 2015-07-31 15:23:42 PDT 
Is this what you were trying to do?
Comment 5 chris fleizach 2015-08-27 11:36:29 PDT 
Comment on attachment 257963 [details]
Patch

is it possible to add a layout test?
thanks
Comment 6 Joseph Pecoraro 2016-09-05 01:19:47 PDT 
Comment on attachment 257963 [details]
Patch

This issue still exists because this never landed. The static analyzer warning still exists. This never landed because there was no test. Any ideas on how to write one?
Comment 7 chris fleizach 2016-09-05 01:22:27 PDT 
Comment on attachment 257963 [details]
Patch

I think a layout test would try to use the accessibility setSelectedRows method with more than 1 item on a non-multi-selectable selection box, then verify that only one row is selected.