Opened 6 years ago

Closed 3 years ago

#16257 closed defect (fixed)

[patch] [cla][pr] dojo.clone() has problem with sparse arrays

Reported by: jonathanpglick Owned by: dylan
Priority: high Milestone: 1.11
Component: Core Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

I've found a problem with cloning arrays when a value is assigned to a key that is greater than the current length. Example:

var a = [1, 2];
a[3] = 3; // [1, 2, undefined, 3]
dojo.clone(a); // [1, 2, 3, 3]

I'm not sure why this is happening. I tested it on Firefox and Chrome and got the same results. Is this the result of how the array is implemented by javascript?

I've attached a patch with a fix that replaces "r.push()" with "r[i] = " in dojo.clone and it seems to fix the problem and the tests still pass. Also included in the patch is a test case in tests/_base/lang.js.

Let me know if this is expected behavior or if there's a reason Array.push() is being used there.

Attachments (1)

clone.patch (876 bytes) - added by jonathanpglick 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by jonathanpglick

Attachment: clone.patch added

comment:1 Changed 6 years ago by bill

Owner: set to Eugene Lazutkin
Status: newassigned

Seems like a reasonable change, especially since the patch updates the automated tests.

comment:2 Changed 4 years ago by bill

#18577 is a duplicate of this ticket.

comment:3 Changed 4 years ago by dylan

Milestone: tbd1.11
Owner: changed from Eugene Lazutkin to dylan

Will need to update the test to Intern, but otherwise looks reasonable.

comment:4 Changed 3 years ago by dylan

Priority: undecidedhigh

comment:5 Changed 3 years ago by dylan

Summary: [patch] [cla] dojo.clone() has problem with sparse arrays[patch] [cla][pr] dojo.clone() has problem with sparse arrays

Pull request created and ready for review at https://github.com/dojo/dojo/pull/194

comment:6 Changed 3 years ago by dylan

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.