Bug 138241

Summary:

AX:setSelectedRows loop is incorrect -- ignores the count

Product:

WebKit

Reporter:

chris fleizach <cfleizach>

Component:

Accessibility

Assignee:

Matthew Daiter <mdaiter>

Status:

NEW

Severity:

Normal

CC:

aboxhall, apinheiro, cdumez, commit-queue, dmazzoni, jcraig, jdiggs, joepeck, mario, mdaiter, samuel_white, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	cfleizach: review-, cfleizach: commit-queue-

chris fleizach

Reported 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 :)

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 attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2014-10-30 23:24:24 PDT

<rdar://problem/18834530>

Radar WebKit Bug Importer

Comment 2 2014-10-30 23:25:14 PDT

<rdar://problem/18834534>

Matthew Daiter

Comment 3 2015-07-31 14:59:00 PDT

Created attachment 257963 [details] Patch

Matthew Daiter

Comment 4 2015-07-31 15:23:42 PDT

Is this what you were trying to do?

chris fleizach

Comment 5 2015-08-27 11:36:29 PDT

Comment on attachment 257963 [details] Patch is it possible to add a layout test? thanks

Joseph Pecoraro

Comment 6 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?

chris fleizach

Comment 7 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.

Note You need to log in before you can comment on or make changes to this bug.