Skip to content

Commit

Permalink
Switch back to using Sizzle.matchesSelector.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeresig
jeresig committed Oct 10, 2010
1 parent eb67d99 commit ba149e7
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
4 changes: 2 additions & 2 deletions speed/filter.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
setTimeout(function(){
name = "filter '" + item + "'";
jQuery("#results").append("<li>" + name + "<ul>" +
"<li>new: " + benchmarkString("$('div').filter('" + item + "')", 1000, name) + "</li>" +
"<li>old: " + benchmarkString("old('div').filter('" + item + "')", 1000, name) + "</li>" +
"<li>new: " + benchmarkString("$('div').filter('" + item + "')", 100, name) + "</li>" +
"<li>old: " + benchmarkString("old('div').filter('" + item + "')", 100, name) + "</li>" +
"</ul></li>");
jQuery("#results").append("<li>single " + name + "<ul>" +
"<li>new: " + benchmarkString("$('#nonexistant').filter('" + item + "')", 1000, name) + "</li>" +
Expand Down
13 changes: 7 additions & 6 deletions src/traversing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ var runtil = /Until$/,
rmultiselector = /,/,
rchild = /^\s*>/,
isSimple = /^.[^:#\[\.,]*$/,
slice = Array.prototype.slice;

var POS = jQuery.expr.match.POS;
slice = Array.prototype.slice,
POS = jQuery.expr.match.POS;

jQuery.fn.extend({
find: function( selector ) {
Expand Down Expand Up @@ -100,13 +99,13 @@ jQuery.fn.extend({
var pos = POS.test( selectors ) ?
jQuery( selectors, context || this.context ) : null;

var ret = [];
ret = [];

for ( var i = 0, j = this.length; i < j; i++ ) {
var cur = this[i];

while ( cur ) {
if ( pos ? pos.index(cur) > -1 : jQuery.find.matches(selectors, cur) ) {
if ( pos ? pos.index(cur) > -1 : jQuery.find.matchesSelector(cur, selectors) ) {
ret.push( cur );
break;

Expand Down Expand Up @@ -229,7 +228,9 @@ jQuery.extend({
expr = ":not(" + expr + ")";
}

return jQuery.find.matches(expr, elems);
return elems.length === 1 ?
jQuery.find.matchesSelector(elems[0], expr) ? [ elems[0] ] : [] :
jQuery.find.matches(expr, elems);
},

dir: function( elem, dir, until ) {
Expand Down

11 comments on commit ba149e7

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running the unit tests, I'm getting:

core: selector state (2, 18, 20)

Died on test #19: jQuery.find.matchesSelector is not a function

Unfortunately, I'm on my way out now and dont have time to write and submit a patch. Sorry.

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to pull in the latest copy of Sizzle.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok cool, I just pulled this down and ran 'make' ... Should the build process be updated to check for new sizzle (and qunit while we're at it)?

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already does - running make runs make init - which pulls in the latest code from Sizzle and QUnit.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, that's what I thought - but I pulled down all the latest commits from jquery master and then ran make, followed by opening the test suites and getting the failure. I will check through all my local files when I get home (sorry, I'm posting from mobile)

@dmethvin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rick, I use the Ant build and it definitely saw the change in Sizzle and rebuilt it.

@csnover
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I ran make, the Sizzle repo was reported as being up-to-date, but sizzle.js was in a modified state such that all of the new changes were removed (running git diff showed basically a reverse diff of the latest Sizzle update). I had to git reset --hard HEAD in src/sizzle to fix it. Dan Heberden also reported this problem and that this solution worked. Not sure if this is an artefact of making changes or if the Makefile is doing something wrong when pulling external dependencies or what.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still getting errors and i definitely have an updated version. The errors had gone away but have returned.... this is a comment i left on the sizzle.js commit ( http://github.com/jeresig/sizzle/commit/b6d5e1761da834fd213ecfb92d5e4c7f278ac2dc ) :

This change wasn't updated in jquery/src/traversing.js

http://github.com/jquery/jquery/blob/master/src/traversing.js#L99

which causes massive unit test failures. I patched my local copy and the tests that were failing are passing, but now there are massive selector related failures across the modules: attributes.js(71, 72, 73), events.js(103,105,106,107,108,109,110,112,113,114) traversing.js(133,136,141,142, 154,156,157,158, 161,162) manipulation.js(199,200,203,204,294, 299,304,309,314,319,324,329,331,333,336,338,342,348)

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://github.com/jeresig/sizzle/commit/b6d5e1761da834fd213ecfb92d5e4c7f278ac2dc

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to clarify for others: Make sure that you run 'make clean' before running 'make' - that seems to help these odd cases.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. I thought I had deleted this comment, as well as the one on sizzle. Apparently, deleting comments doesn't actually do anything except for remove them from the DOM.

Please sign in to comment.