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 comment:1
milestone: | 1.9.0 → 2.0.0 |
---|
Changed November 05, 2012 11:56PM UTC by comment:2
resolution: | → notabug |
---|---|
status: | new → closed |
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.