array.splice() with 1 argument not working

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jan Boeijink]

splice() with 1 element is not working in Webkit. Compare behaviour with Firefox.
array.splice(index) should remove all items starting from index. 
See attachment.

[Jan Boeijink]

Created attachment 18332 [details]
array.splice() with one element

[David Kilzer (:ddkilzer)]

Same issue occurs with Safari 2.0.4 (419.3) and Safari 3.0.4 (523.12.2) with original WebKit(s) on Mac OS  X 10.4.11 (8S165).

[Alexey Proskuryakov]

According to <http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Objects:Array:splice>, this is a non-standard Mozilla extension (not to imply that we don't want to support it).

[David Kilzer (:ddkilzer)]

Safari 3.0.4, MSIE 7 and Opera 9.24 follow the standard, and thus splice() with one argument does nothing in these browsers.

[Alexey Proskuryakov]

Created attachment 18350 [details]
proposed fix

I have a fix anyway, a reviewer can decide whether we want this.

[David Kilzer (:ddkilzer)]

Comment on attachment 18350 [details]
proposed fix

r=me

Does this change impact SunSpider performance?  The following two tests use splice:

SunSpider/tests/date-format-tofte.js
SunSpider/tests/string-unpack-code.js

[Darin Adler]

Comment on attachment 18350 [details]
proposed fix

+    if (!args.size())

+    if (args.size() > 1)

Our usual design is to always consider undefined arguments rather than checking number of arguments. So rather than checking for lack of arguments, we detect the case where arguments are undefined, treating a smaller number of arguments identically to a call with explicit "undefined" for th extra arguments.

I'd like to see test cases that pass undefined to make this clear one way or another. Normally I consider it forbidden to actually look at args.size(), but if we're trying to match the Mozilla behavior it's possible it's needed here.

[Alexey Proskuryakov]

(In reply to comment #7)
The included test checks for this, if I understood you correctly:

+shouldBe("arr.splice()", "undefined")
+shouldBe("arr", "['a','b','c','d']");
+shouldBe("arr.splice(undefined)", "['a','b','c','d']")
+shouldBe("arr", "[]");

(similar for the second argument). Indeed, I was trying to match Mozilla behavior.

If anyone wonders why I haven't landed this yet - it's because I didn't have the time to build release and test performance.

[Jan Boeijink]

This may be off-topic, but Darin's comment reminded me that before I found this little omission, I wondered if this could be possible:

array1 = ['a','b','c','d'];
array2 = array1.splice(2,undefined,'e','f','g');

--> array1 = ['a','b','e','f','g'];
--> array2 = ['c','d'];

In other words, remove unknown amount of elements starting from position 2 like in splice(2), and add some elements. Ofcourse there are other ways...

The actual result (in Webkit and Firefox) is:
 --> array1 = ['a','b','e','f','g','c','d'];

[Alexey Proskuryakov]

Committed revision 29459.