.filter(array) bug, and patch

[brandon]

From Fil on list:

Hello,

I've encountered a bug in .filter(array)

however the bug is only visible in a complex script, and within th Firebug Inspector (but I can assure you it breaks my script), and I was not able to write a simple test... so you might prefer to ignore me :)

I can provide the complex test if absolutely needed, but it will need more work than just testing this patch against the test suite, I guess.

In any case, here is the patch:

filter: function(t) {
               return this.pushStack(
                       t.constructor == Array &&
-                       jQuery.map(this,function(a){
+                       jQuery.grep(this,function(a){
                               for ( var i = 0; i < t.length; i++ )
                                       if ( jQuery.filter(t[i],[a]).r.length )
-                                               return a;
+                                               return true;
                               return false;
                       }) ||

The idea of a .filter() is to remove nodes that don't match, not to insert 'false' instead. In my tests, when I use Firebug I clearly see that the object I receive is something like the (crazy) following:

       length: 1
       0: node0
       1: false
       2: node2

-- Fil

[brandon]

From Jorn on list:

I wonder if the filter(Array<String>) method is really useful, why not just use this: $("h1").filter(".one, .nomatch").css("background", "red"); Should be pretty much the same.

[brandon]

From Sam on list:

Couldn't filter still take an array, but just convert it to a string internally using join (which I thought it would have done anyway)?

Although the user could do that manually, i.e. filter([".one",".two"].join(','))

[brandon]

From Andrea on list:

the fix is much simpler:

t.constructor == Array &&
jQuery.map(this,function(a){
       for ( var i = 0; i < t.length; i++ )
               if ( jQuery.filter(t[i],[a]).r.length )
                       return a;
-       return false;
+       return null;
}) ||

I've tested with coreTest.js and your example and it works.

[brandon]

From Fil on list:

I agree, but it's really grep that we want to do here, in my opinion. Either way, it will be fixed :)

Someone mentioned .filter(".one,.two"), but it has exactly the same bug.

[bertrand@…]

Actually, .filter(".one, .three") does not make any error but will filter only the "one" class as it was .filter(".one")

[joern]

Thanks folks, fixed in SVN.