Opened 10 years ago

Closed 7 years ago

#9942 closed defect (wontfix)

[patch] [cla] Dojo array iteration methods deviate from spec on sparse arrays

Reported by: Ben Lowery Owned by: Kris Zyp
Priority: high Milestone: 2.0
Component: Core Version: 1.3.2
Keywords: Cc: Eugene Lazutkin
Blocked By: Blocking:

Description

dojo.forEach doesn't follow the spec at https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Objects/Array/forEach and incorrectly fires the callback provided for undefined keys in a sparse array. For example:

var arr = [];
arr[1] = "foo"'
arr[5] = "bar";

// this prints 6 times
dojo.forEach(arr, function(n,i) { console.log("%d: %s", i, n); });

// this prints twice
arr.forEach(function(n, i) { console.log("%d: %s", i, n); });

dojo's forEach should be checking to ensure that each key from the for loop is present in the array before firing the callback. See the example implementation provided at https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Objects/Array/forEach#Compatibility for further illustration.

Attachments (1)

sparse_array_iteration.patch (4.4 KB) - added by Ben Lowery 10 years ago.
Patch that fixes sparse array iteration

Download all attachments as: .zip

Change History (15)

Changed 10 years ago by Ben Lowery

Patch that fixes sparse array iteration

comment:1 Changed 10 years ago by Ben Lowery

Here's a patch that fixes the iteration methods, along with supporting tests.

comment:2 Changed 10 years ago by bill

Summary: dojo.forEach incorrectly walks sparse arrays[patch] [cla] dojo.forEach incorrectly walks sparse arrays

comment:3 Changed 10 years ago by Eugene Lazutkin

Cc: Eugene Lazutkin added

comment:4 Changed 10 years ago by James Burke

Milestone: tbd2.0

As discussed on list, we will defer this to 2.0 when we can also consider delegating back to native methods.

comment:5 Changed 10 years ago by bill

The discussion is in http://thread.gmane.org/gmane.comp.web.dojo.devel/11475.

Let's be careful whether or not we do in 2.0. Although the current behavior is different than Array.forEach(), it shouldn't be considered a bug. Enhancing forEach() to walk sparse arrays will slow down all versions of IE, and unclear of the effect on other browsers (see [10200]).

comment:6 Changed 10 years ago by James Burke

(In [20229]) Refs #9942, adds a note to dojo.forEach about operating over sparse arrays, and updates devmo links for the different array methods.

comment:7 Changed 10 years ago by Eugene Lazutkin

All array method docs should be updated --- the standard allows sparse arrays on all of them, we support none.

comment:8 Changed 10 years ago by Ben Lowery

(In [20232]) refs #9942. Adds in doc updates for the other array iteration methods indicating our difference with sparse arrays.

comment:9 Changed 10 years ago by Ben Lowery

(In [20233]) refs #9942. Minor doc consistency fixes for the sparse array warning.

comment:10 Changed 10 years ago by Ben Lowery

Updated the remaining docs.

comment:11 Changed 10 years ago by Ben Lowery

Summary: [patch] [cla] dojo.forEach incorrectly walks sparse arrays[patch] [cla] Dojo array iteration methods deviate from spec on sparse arrays

Updating the title a bit to reflect what's really the matter.

comment:12 Changed 8 years ago by Chris Mitchell

Owner: anonymous deleted

comment:13 Changed 7 years ago by bill

Component: GeneralCore
Owner: set to Kris Zyp
Status: newassigned

comment:14 Changed 7 years ago by Kitson Kelly

Resolution: wontfix
Status: assignedclosed

Dojo 2.0 will only support ES5 user agents, the array routines (which pre-dated ES5) will not be present in Dojo 2.0, so closing the ticket.

Note: See TracTickets for help on using tickets.