Search and Top Navigation
#5985 closed bug (notabug)
Opened August 24, 2010 05:13AM UTC
Closed August 24, 2010 11:24AM UTC
Last modified October 11, 2012 09:15PM UTC
_removeCurrentsFromItems: function() problem
| Reported by: | samo83 | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | ui.sortable | Version: | 1.8.4 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Hi, I'm not sure if this is the right place but I am having problems with the _removeCurrentsFromItems function, reported as being on line 3385 by Firebug. I can follow the code below in Firebug and it is happy until it tries a subsequent iteration over this.items.
_removeCurrentsFromItems: function() {
var list = this.currentItem.find(":data(sortable-item)");
for (var i=0; i < this.items.length; i++) {
for (var j=0; j < list.length; j++) {
if(list[j] == this.items[i].item[0])
this.items.splice(i,1);
};
};
},
In my test example, to begin with, this.items.length = 12 and list.length = 15. By the time list has been iterated over, this.items only contains two items and I get the error this.items[i] is undefined. Can javascript deal with an array changing in size while being iterated over? Any help would be much appreciated (by the way I am nesting sortables, which could be a possible explanation). Cheers, Sam
Attachments (0)
Change History (3)
Changed August 24, 2010 11:24AM UTC by comment:1
| component: | ui.core → ui.sortable |
|---|---|
| resolution: | → invalid |
| status: | new → closed |
Changed November 01, 2010 04:54PM UTC by comment:2
This portion of code is clearly erroneous - while looking at something else I spotted it. It's iterating over this.items from 0 to this.items.length whilst potentially removing entries from this.items as it goes, which would mean that if the "if" clause was ever true it would skip over items and also overrun. The outer iteration on this.items.length should do what other iterations of a similar ilk do:
for (var i = this.items.length - 1; i >= 0; i--) {
Secondly it should do a break after calling this.items.splice(i,1) as there's no point continuing to check "list[j]" against this.items[i] since you've just removed the item you were checking in that run through of "list".
I was tempted to try write a test case for it myself as any code which triggers a match in the "if" would hit the problem, but there is no documentation in the code saying what on earth _removeCurrentsFromItems is for or what circumstances it would go off. I deduced that it would only get called if you had some sort of nesting going on. The code appears to have come in with r487 to support ui.tree - in fact if you create a nested set of UL's (i.e. have ULs with LIs which have ULs which have LIs), make the outermost UL sortable and then as the "items" selector pass in just "li" then the code will trigger when you drag LIs with child lists (and the splice call will go off too). I think the error that samo83 reported of it actually triggering a javascript error will only happen if you get matches when i==items.length, so it may depend purely on luck with the order of things in items.
Changed October 11, 2012 09:15PM UTC by comment:3
| milestone: | TBD |
|---|
Milestone TBD deleted
Please provide a reduced test case showing the problem.