Traversing: Never let .closest() match positional selectors

Fixes gh-2796 Close gh-2818
jquery · a268f5225cad9ab380494e61a10105cc9eb107e7 · gibson042 committed with Timmy Willison Jan 12, 2016 · Jan 12, 2016 · a268f52
1 parent 0e2f8f9
commit a268f5225cad9ab380494e61a10105cc9eb107e7
Unified Split

Showing with 31 additions and 17 deletions.

+14 −13 src/traversing.js

+17 −4 test/unit/traversing.js
diff --git a/src/traversing.js b/src/traversing.js
@@ -39,23 +39,24 @@ jQuery.fn.extend( {
 			i = 0,
 			l = this.length,
 			matched = [],
-			pos = rneedsContext.test( selectors ) || typeof selectors !== "string" ?
-				jQuery( selectors, context || this.context ) :
-				0;
+			targets = typeof selectors !== "string" && jQuery( selectors );
 
-		for ( ; i < l; i++ ) {
-			for ( cur = this[ i ]; cur && cur !== context; cur = cur.parentNode ) {
+		// Positional selectors never match, since there's no _selection_ context
+		if ( !rneedsContext.test( selectors ) ) {
+			for ( ; i < l; i++ ) {
+				for ( cur = this[ i ]; cur && cur !== context; cur = cur.parentNode ) {
 
-				// Always skip document fragments
-				if ( cur.nodeType < 11 && ( pos ?
-					pos.index( cur ) > -1 :
+					// Always skip document fragments
+					if ( cur.nodeType < 11 && ( targets ?
+						targets.index( cur ) > -1 :
 
-					// Don't pass non-elements to Sizzle
-					cur.nodeType === 1 &&
-						jQuery.find.matchesSelector( cur, selectors ) ) ) {
+						// Don't pass non-elements to Sizzle
+						cur.nodeType === 1 &&
+							jQuery.find.matchesSelector( cur, selectors ) ) ) {
 
-					matched.push( cur );
-					break;
+						matched.push( cur );
+						break;
+					}
 				}
 			}
 		}
diff --git a/test/unit/traversing.js b/test/unit/traversing.js
@@ -323,7 +323,7 @@ QUnit[ jQuery.find.compile ? "test" : "skip" ]( "filter() with positional select
 } );
 
 QUnit.test( "closest()", function( assert ) {
-	assert.expect( 13 );
+	assert.expect( 14 );
 
 	var jq;
 
@@ -344,6 +344,12 @@ QUnit.test( "closest()", function( assert ) {
 	// Test on disconnected node
 	assert.equal( jQuery( "<div><p></p></div>" ).find( "p" ).closest( "table" ).length, 0, "Make sure disconnected closest work." );
 
+	assert.deepEqual(
+		jQuery( "#firstp" ).closest( q( "qunit-fixture" ) ).get(),
+		q( "qunit-fixture" ),
+		"Non-string match target"
+	);
+
 	// Bug #7369
 	assert.equal( jQuery( "<div foo='bar'></div>" ).closest( "[foo]" ).length, 1, "Disconnected nodes with attribute selector" );
 	assert.equal( jQuery( "<div>text</div>" ).closest( "[lang]" ).length, 0, "Disconnected nodes with text and non-existent attribute selector" );
@@ -355,10 +361,17 @@ QUnit.test( "closest()", function( assert ) {
 } );
 
 QUnit[ jQuery.find.compile ? "test" : "skip" ]( "closest() with positional selectors", function( assert ) {
-	assert.expect( 2 );
+	assert.expect( 3 );
 
-	assert.deepEqual( jQuery( "#qunit-fixture" ).closest( "div:first" ).get(), [], "closest(div:first)" );
-	assert.deepEqual( jQuery( "#qunit-fixture div" ).closest( "body:first div:last" ).get(), q( "fx-tests" ), "closest(body:first div:last)" );
+	assert.deepEqual( jQuery( "#qunit-fixture" ).closest( "div:first" ).get(), [],
+		"closest(div:first)" );
+	assert.deepEqual( jQuery( "#qunit-fixture div" ).closest( "body:first div:last" ).get(), [],
+		"closest(body:first div:last)" );
+	assert.deepEqual(
+		jQuery( "#qunit-fixture div" ).closest( "body:first div:last", document ).get(),
+		[],
+		"closest(body:first div:last, document)"
+	);
 } );
 
 QUnit.test( "closest(jQuery)", function( assert ) {