function .not() and selector :not() don't work on complex expressions

[davidserduke]

There was a posting on the dev boards

 http://groups.google.com/group/jquery-dev/browse_frm/thread/dc26671e7e658e68

about not() failing to work in certain situations. Bryce Lohr found the bug and put together this test case:

 http://gearheadsoftware.com/jQueryNotBug.html

It appears that while the function not() works in simple cases like

not(".hilite")

it fails in more complex situations like

not("input[name='radiobuttons']")

[davidserduke]

A shorter test case

[davidserduke]

My take is the problem stems from the fact that jQuery's selector engine primarily works on an AND basis. If you write the expression "div.hilite" that means it selects a (div AND .hilite). The exception is, of course, the comma but that looks special cased.

So the engine treats not by putting a !(expr) around it. This works fine if you have not(":first") because you have only one term in the expression.

!(a) == !a

But if you have another term like not("div:first") then you have

!(a && b)

And internally it looks like jQuery is still using progressive refinement but attaching the ! to each term which gives

!a && !b

But unfortunately they aren't the same. Rather it should be

!a || !b

I'm not sure what the actual solution should be, but the easy solution is to just create a jQuery object with the expression and then run it through not() which just removes anything in the original that is also in the

jQuery.fn.xNot = function (expr) {
  return this.pushStack( this.not(  jQuery(expr) ) );
};

which I believe would integrate in to the 1.2.1 code like this

not: function(t) {
  if (t.constructor == String) 
    return this.not(jQuery(t));
  else 
    return this.pushStack(
      jQuery.grep(this, function(a) {
        return ( t.constructor == Array || t.jquery )
          ? jQuery.inArray( a, t ) < 0
          : a != t;
      })
    );
},

Unfortunately the problem also exists in :not() but I think for the :not() special case it can be changed to

r = jQuery(r).not(m[3]).get();

Obviously for this stuff there will be a speed hit but I think it should work.

I created a patch for svn (similar to the above code) that fixes the test cases.

[wizzud]

I agree entirely with David's analysis of the problem. However I would like to propose a very slightly amended solution.

The only problem with David's solution - as I see it - is that the

return this.not(jQuery(t));

line within the not() function creates the jQuery object of t with the context of the entire document.

Since not() is the inverse of filter(), I would propose changing not() to

not: function( selector ) {
	if(selector.constructor == String)
		selector = jQuery.multiFilter( selector, this );
	return this.pushStack( jQuery.invert( selector, this ) );
},

... changing the :not() filter processing to

r = jQuery.invert( jQuery.filter(m[3], r).r, r );

... extending jQuery with

// remove all 'elems' elements from the 'from' collection of elements...
invert: function( elems, from ) {
	var ret = jQuery.grep( from, function(elem) {
		return from.constructor == Array || from.jquery ?
			jQuery.inArray( elem, elems ) < 0 :
			elem != from;
		});
	return ret;
},

... and removing all 'not' arguments and processing (excluding :not()!) from...

grep() multiFilter() classFilter() filter()

This has the advantage of simplifying the 4 functions above and completely abstracting the not() functionality from filtering such that if filtering works correctly and as optimally as possible then so does not().

It makes the selectors available to not() exactly the same as those available to filter(), avoids recursion of not(), keeps context narrowed down where possible, and reduces the overhead on a not() call to a single additional grep().

The addition of the invert() method to jQuery is not necessary, but saves a few bytes since the grep call it contains would be used in both the not() function and the :not() filter processing (in my solution), and it keeps the code simple and readable!.

I'm sure it can be improved upon by the experts, but it seems to do the job.

[davidserduke]

Fixed in [4143]. Thanks wizzud for the suggestions and the code! It pretty much went straight in as you wrote it.