Skip to main content

Search and Top Navigation

#6818 closed bug (notabug)

Opened January 06, 2011 04:54PM UTC

Closed November 05, 2012 11:56PM UTC

_removeCurrentsFromItems modifies a list it is iterating over with a for(i=0...) style loop

Reported by: Skaffen Owned by:
Priority: minor Milestone: 2.0.0
Component: ui.sortable Version: 1.8.7
Keywords: Cc:
Blocked by: Blocking:
Description

I posted a comment on this on the closed ticket #5985 but I guess no-one saw it.

The outer of the loop is:

for (var i=0; i < this.items.length; i++) {

Which has an inner loop which does an iteration and can do:

if(list[j] == this.items[i].item[0])

this.items.splice(i,1);

So the meaning of items[i] will change if the splice goes off, which means that this loop can skip doing some of the comparisons it should be doing and potentially even have this.items[i] cause an error if the list shrinks below what the current [i] value is. I've no idea really what _removeCurrentsFromItems is for, so I can't cook up a test case - hopefully someone who knows more about the purpose of this function can cook up a test case which will demonstrate the bug.

The fix to the code would be iterate backwards like other similar code does, e.g. for (var i = this.items.length - 1; i >= 0; i--), and to break out of the inner 'j' loop if a match is found that triggers this.items.splice(i,1).

Attachments (0)
Change History (2)

Changed October 11, 2012 02:54PM UTC by scottgonzalez comment:1

milestone: 1.9.02.0.0

Changed November 05, 2012 11:56PM UTC by mikesherov comment:2

resolution: → notabug
status: newclosed

Thanks for contributing! The code in reference no longer exists as is. Since there's no test case (as was asked for), I need to mark this as notabug.