Fix #13721. Filter before .remove() loop so positionals work. Close gh-1221.

diff --git a/src/manipulation.js b/src/manipulation.js
index 5cfc853be3eb031cdff6ced646c752ca2a93f9d5..f3c729fdfb04738d03cd1a13516775d95be2b97d 100644 (file)
--- a/src/manipulation.js
+++ b/src/manipulation.js
@@ -72,23 +72,21 @@ jQuery.fn.extend({
        // keepData is for internal use only--do not document
        remove: function( selector, keepData ) {
                var elem,
-                       i = 0,
-                       l = this.length;
+                       elems = selector ? jQuery.filter( selector, this ) : this,
+                       i = elems.length;
 
-               for ( ; i < l; i++ ) {
-                       elem = this[ i ];
+               while ( i-- ) {
+                       elem = elems[ i ];
 
-                       if ( !selector || jQuery.filter( selector, [ elem ] ).length > 0 ) {
-                               if ( !keepData && elem.nodeType === 1 ) {
-                                       jQuery.cleanData( getAll( elem ) );
-                               }
+                       if ( !keepData && elem.nodeType === 1 ) {
+                               jQuery.cleanData( getAll( elem ) );
+                       }
 
-                               if ( elem.parentNode ) {
-                                       if ( keepData && jQuery.contains( elem.ownerDocument, elem ) ) {
-                                               setGlobalEval( getAll( elem, "script" ) );
-                                       }
-                                       elem.parentNode.removeChild( elem );
+                       if ( elem.parentNode ) {
+                               if ( keepData && jQuery.contains( elem.ownerDocument, elem ) ) {
+                                       setGlobalEval( getAll( elem, "script" ) );
                                }
+                               elem.parentNode.removeChild( elem );
                        }
                }
 

diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index 012a0efc9a4f101ccae8a6358f0374d952d3ac42..6093dbfb662469b32d0632af854e0cb0e457aa29 100644 (file)
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -1713,7 +1713,8 @@ test( "clone()/html() don't expose jQuery/Sizzle expandos (#12858)", function()
 });
 
 var testRemove = function( method ) {
-       var first = jQuery("#ap").children().first();
+       var markup, div,
+               first = jQuery("#ap").children().first();
 
        first.data("foo", "bar");
 
@@ -1731,6 +1732,18 @@ var testRemove = function( method ) {
        jQuery("#ap").children()[ method ]("a, code");
        equal( jQuery("#ap").children().length, 0, "Check multi-filtered remove" );
 
+       // Positional and relative selectors
+       markup = "<div><span>1</span><span>2</span><span>3</span><span>4</span></div>";
+       div = jQuery( markup );
+       div.children().remove("span:nth-child(2n)");
+       equal( div.text(), "13", "relative selector in " + method );
+       div = jQuery( markup );
+       div.children().remove("span:first");
+       equal( div.text(), "234", "positional selector in " + method );
+       div = jQuery( markup );
+       div.children().remove("span:last");
+       equal( div.text(), "123", "positional selector in " + method );
+
        // using contents will get comments regular, text, and comment nodes
        // Handle the case where no comment is in the document
        ok( jQuery("#nonnodes").contents().length >= 2, "Check node,textnode,comment remove works" );
@@ -1743,7 +1756,7 @@ var testRemove = function( method ) {
        }
 };
 
-test( "remove()", 8, function() {
+test( "remove()", 11, function() {
        testRemove("remove");
 });
 
@@ -1762,7 +1775,7 @@ test( "remove() event cleaning ", 1, function() {
        cleanUp.remove();
 });
 
-test( "detach()", 8, function() {
+test( "detach()", 11, function() {
        testRemove("detach");
 });