This is not showing up in core as an error, but if the Array prototype has been added to, this will cause the iterating code to run over properties (and methods) of the Array, as opposed to just the elements. It's bad form in JavaScript to use the for...in construct when working with arrays (should only be used for objects).

Files: 
CommentFileSizeAuthor
#10 tabledrag_iterator-800968-d6-10.patch578 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#7 tabledrag-for-800968-7.patch.txt511 bytesBig Z
#2 800968_tabledrag-for_2.patch551 bytesksenzee
PASSED: [[SimpleTest]]: [MySQL] 20,356 pass(es).
[ View ]
#1 tabledrag_iterator.patch1.34 KBJacobSingh
PASSED: [[SimpleTest]]: [MySQL] 20,354 pass(es).
[ View ]
tabledrag_iterator.patch3.44 KBJacobSingh
PASSED: [[SimpleTest]]: [MySQL] 20,340 pass(es).
[ View ]

Comments

StatusFileSize
new1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 20,354 pass(es).
[ View ]

Oops, snuck some extra changes in there.

StatusFileSize
new551 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,356 pass(es).
[ View ]

A simpler fix is just to construct the for loop correctly.

Title:tabledrag.js should be using $().each instead of for...in when iterating over an array.tabledrag.js should not use for...in to iterate over an array

Guess I should retitle the issue, too.

Status:Needs review» Reviewed & tested by the community

Yep. Better Javascript!

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Patch (to be ported)
StatusFileSize
new511 bytes

Could this please be back ported to the 6.x branch. Patch attached.

Many thanks!

subscribe

+1 for backport to D6

Status:Patch (to be ported)» Needs review
StatusFileSize
new578 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Re-posting patch from #7. No credit please!

Component:block.module» base system
Priority:Minor» Major
Status:Needs review» Reviewed & tested by the community

Super, perfect fix for my ageing Drupal 6 site.
At present its preventing admin forms from working in Chrome so a bit more than a 'minor' issue I think.
The bug is also not just in the block.module - it also affects the taxonomy admin, webform field designer, CCK field editor etc.

Status:Reviewed & tested by the community» Fixed

Looks good. Thanks, committed, pushed.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

I usually check the patches before I update my Drupal and I ran across this issue and fix.

I would recommend taking it one step further and look at reducing the number of gratuitous .length calls with the hopes of improving the speed of tabledrag.js further. From my simple test (profiling patch included in issue), it could reduce the number of calls to .length from ~30,000+ to roughly 450 for a single drag.

Take a look
#1571814: tableDrag.js, reduce the number of calls of .length from 33478 to 455