Skip to main content

Bug Tracker

Side navigation

#1854 closed bug (fixed)

Opened October 27, 2007 11:23PM UTC

Closed December 13, 2007 10:28PM UTC

Last modified March 15, 2012 12:52AM UTC

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

Reported by: davidserduke Owned by:
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: Cc:
Blocked by: Blocking:
Description

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']")

Attachments (2)
  • 1854.diff (1.3 KB) - added by davidserduke October 27, 2007 11:45PM UTC.
  • jquery_test.html (1.6 KB) - added by davidserduke October 27, 2007 11:45PM UTC.

    A shorter test case

Change History (3)

Changed October 27, 2007 11:51PM UTC by davidserduke comment:1

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.

Changed October 28, 2007 12:50PM UTC by wizzud comment:2

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.

Changed December 13, 2007 10:28PM UTC by davidserduke comment:3

resolution: → fixed
status: newclosed

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