Last modified: 2009-08-24 17:22:57 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T22376, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 20376 - Replace fragile "for ... in" loop in wikibits.js with proper iteration over index array
Replace fragile "for ... in" loop in wikibits.js with proper iteration over i...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Normal minor (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-24 16:45 UTC by Sean Prunka
Modified: 2009-08-24 17:22 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Sean Prunka 2009-08-24 16:45:33 UTC
In the JavaScript file wikibits.js there is an iteration over an indexed array using a "for ... in" style loop. This style of loops works on arrays when they are only being used as arrays and have not been extended in anyway. However, while this does work, it is not the proper use of the "for ... in" style loop. This iterative loops is design for stepping through all of the properties of an object. Since an array is an Object, it can contain properties other than the simple indexes. When you only want to step through the numbered indexes of an array, you should use the "for (var x = 0; x < array.length; x++)" loop.

The only reason this comes up is that in some of the extensions we've written for our wikis we make use of prototype.js (which is known to extend all objects, including arrays) ... which in turn causes a JavaScript error ta[id][0] is undefined.

While neither party is in the right here, and debates can, have and likely will be held on which is more wrong, I offer you this simple solution to fix your own end, so that you may be more righteous and zealous in any arguments you may have against prototype.

In the core code of 1.13.2 (unsure of where it may lie in other releases)
wikibits.js approx. line 338 the current code reads:
     for (var id in ta) {
If you change it to read:
    for(var id = 0 ; id < ta.length ; id++) {

Everything will still work as it has in the past, but it will not throw errors at those extending the array object either.

your attention to this matter is appreciated, even if you end up scoffing at us and not fixing it.
Comment 1 Brion Vibber 2009-08-24 16:48:32 UTC
Clarified summary.
Comment 2 Daniel Friesen 2009-08-24 17:22:57 UTC
Fixed r55555.
I wouldn't really consider it debatable, for..in is for object iteration, for(var i..., arr.forEach are for array iteration, any good js developer will tell you that. for..in and for each..in aren't acceptable for array iteration unless you are in a server environment with an iterator on the array to make it behave right like I am.
It's a shame we don't have some moz/ES5 compat methods in though, then we could happily use ES5's .forEach() in code which moz already has native support for.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links