Generic jQuery.merge and safer jQuery.makeArray(nodeList).

* Made jQuery.marge more generic supporting null and undefined values, and supporting array like objects as results. Fixes #5527. * Made jQuery.makeArray(nodeList) more safer using jQuery.merge internally. Fixes #5528. * Added a second "results" argument to jQuery.makeArray for internal usage to avoid intermediary arrays in init.
jquery · Nov 25, 2009 · ac00fe5 · ac00fe5
1 parent b30af34
commit ac00fe5
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 19 deletions.
diff --git a/src/core.js b/src/core.js
@@ -132,9 +132,9 @@ jQuery.fn = jQuery.prototype = {
 			this.context = selector.context;
 		}
 
-		return this.setArray(jQuery.isArray( selector ) ?
+		return jQuery.isArray( selector ) ?
-			selector :
+			this.setArray( selector ) :
-			jQuery.makeArray(selector));
+			jQuery.makeArray( selector, this );
 	},
 
 	// Start with an empty selector
@@ -419,19 +419,16 @@ jQuery.extend({
 		return (text || "").replace( rtrim, "" );
 	},
 
-	makeArray: function( array ) {
+	// results is for internal usage only
-		var ret = [], i;
+	makeArray: function( array, results ) {
+		var ret = results || [];
 
 		if ( array != null ) {
-			i = array.length;
-
 			// The window, strings (and functions) also have 'length'
-			if ( i == null || typeof array === "string" || jQuery.isFunction(array) || array.setInterval ) {
+			if ( array.length == null || typeof array === "string" || jQuery.isFunction(array) || array.setInterval ) {
-				ret[0] = array;
+				push.call( ret, array );
 			} else {
-				while ( i ) {
+				jQuery.merge( ret, array );
-					ret[--i] = array[i];
-				}
 			}
 		}
 
@@ -453,12 +450,21 @@ jQuery.extend({
 	},
 
 	merge: function( first, second ) {
-		// We have to loop this way because IE & Opera overwrite the length
+		var pos, i = second.length;
-		// expando of getElementsByTagName
-		var i = 0, elem, pos = first.length;
 
-		while ( (elem = second[ i++ ]) != null ) {
+		// We have to get length this way when IE & Opera overwrite the length
-			first[ pos++ ] = elem;
+		// expando of getElementsByTagName
+		if ( i && i.nodeType ) {
+			for ( i = 0; second[i]; ++i ) {}
+		}
+
+		pos = i + first.length;
+
+		// Correct length for non Arrays
+		first.length = pos;
+
+		while ( i ) {
+			first[ --pos ] = second[ --i ];
 		}
 
 		return first;

diff --git a/test/unit/core.js b/test/unit/core.js
@@ -500,7 +500,7 @@ test("index(Object|String|undefined)", function() {
 });
 
 test("jQuery.merge()", function() {
-	expect(6);
+	expect(8);
 
 	var parse = jQuery.merge;
 
@@ -514,6 +514,10 @@ test("jQuery.merge()", function() {
 
 	// Fixed at [5998], #3641
 	same( parse([-2,-1], [0,1,2]), [-2,-1,0,1,2], "Second array including a zero (falsy)");
+
+	// After fixing #5527
+	same( parse([], [null, undefined]), [null, undefined], "Second array including null and undefined values");
+	same( parse({length:0}, [1,2]), {length:2, 0:1, 1:2}, "First array like");
 });
 
 test("jQuery.extend(Object, Object)", function() {
@@ -554,7 +558,7 @@ test("jQuery.extend(Object, Object)", function() {
 
 	var myKlass = function() {};
 	var customObject = new myKlass();
-	var optionsWithCustomObject = { foo: { date: new customObject } };
+	var optionsWithCustomObject = { foo: { date: customObject } };
 	empty = {};
 	jQuery.extend(true, empty, optionsWithCustomObject);
 	ok( empty.foo && empty.foo.date === customObject, "Custom objects copy correctly (no methods)" );