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)
Change History (3)
Changed October 27, 2007 11:51PM UTC by comment:1
Changed October 28, 2007 12:50PM UTC by 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 comment:3
resolution: | → fixed |
---|---|
status: | new → closed |
Fixed in [4143]. Thanks wizzud for the suggestions and the code! It pretty much went straight in as you wrote it.
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.
But if you have another term like not("div:first") then you have
And internally it looks like jQuery is still using progressive refinement but attaching the ! to each term which gives
But unfortunately they aren't the same. Rather it should be
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
which I believe would integrate in to the 1.2.1 code like this
Unfortunately the problem also exists in :not() but I think for the :not() special case it can be changed to
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.