Skip to main content

Bug Tracker

Side navigation

#2002 closed bug (fixed)

Opened December 04, 2007 05:24PM UTC

Closed December 06, 2007 07:51PM UTC

Optimization in add() breaks API, undefined as argument throws error

Reported by: joern Owned by:
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: add Cc:
Blocked by: Blocking:
Description

In the latest revision, add() was optimized to check for String arguments (selector.constructor == String). This breaks compability with existing code that relies on the 1.2.1 (I guess since 1.0) behaviour to silently ignore when undefined is passed as an argument.

The validation plugin relies on the old behaviour - if the new one intended behaviour, I need to release an update that handles the change before 1.2.2 gets out. Please give me a note in that case.

Testcase:

Index: test/unit/core.js
===================================================================
--- test/unit/core.js	(revision 4012)
+++ test/unit/core.js	(working copy)
@@ -183,8 +183,8 @@
 	ok( $("p").get(0) == document.getElementById("firstp"), "Get A Single Element" );
 });
 
-test("add(String|Element|Array)", function() {
-	expect(7);
+test("add(String|Element|Array|undefined)", function() {
+	expect(8);
 	isSet( $("#sndp").add("#en").add("#sap").get(), q("sndp", "en", "sap"), "Check elements from document" );
 	isSet( $("#sndp").add( $("#en")[0] ).add( $("#sap") ).get(), q("sndp", "en", "sap"), "Check elements from document" );
 	ok( $([]).add($("#form")[0].elements).length >= 13, "Check elements from array" );
@@ -196,6 +196,9 @@
 	var x = $([]).add("<p id='x1'>xxx</p>").add("<p id='x2'>xxx</p>");
 	ok( x[0].id == "x1", "Check on-the-fly element1" );
 	ok( x[1].id == "x2", "Check on-the-fly element2" );
+	
+	var notDefined;
+	equals( $([]).add(notDefined).length, 0, "Check that undefined adds nothing." );
 });
 
 test("each(Function)", function() {

Patch:

Index: src/core.js
===================================================================
--- src/core.js	(revision 4012)
+++ src/core.js	(working copy)
@@ -340,6 +340,8 @@
 	},
 
 	add: function( selector ) {
+		if (!selector)
+			return this;
 		return this.pushStack( jQuery.merge( 
 			this.get(),
 			selector.constructor == String ? 
Attachments (0)
Change History (2)

Changed December 06, 2007 02:32PM UTC by john comment:1

Looks good to me - feel free to commit it, Joern!

Changed December 06, 2007 07:51PM UTC by joern comment:2

resolution: → fixed
status: newclosed

Fixed in [4041]