Bug Tracker

#446 closed bug (fixed)

Opened November 28, 2006 05:20PM UTC

Closed November 29, 2006 08:22PM UTC

.filter(array) bug, and patch

Reported by: brandon Owned by:
Priority: major Milestone:
Component: core Version:
Keywords: Cc:
Blocked by: Blocking:
Description

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

Attachments (0)
Change History (6)

Changed November 28, 2006 05:22PM UTC by brandon comment:1

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.

Changed November 28, 2006 05:22PM UTC by brandon comment:2

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(','))

Changed November 28, 2006 05:23PM UTC by brandon comment:3

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.

Changed November 28, 2006 05:23PM UTC by brandon comment:4

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.

Changed November 28, 2006 06:07PM UTC by bertrand@tog comment:5

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

Changed November 29, 2006 08:22PM UTC by joern comment:6

resolution: → fixed
status: new → closed

Thanks folks, fixed in SVN.