Opened 12 years ago
Closed 10 years ago
#6818 closed bug (notabug)
_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).
Change History (2)
comment:1 Changed 10 years ago by
Milestone: | 1.9.0 → 2.0.0 |
---|
comment:2 Changed 10 years ago by
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.