Bug Tracker

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#7263 closed bug (worksforme)

jQuery.is(':first') not checking correctly

Reported by: qvprof@… Owned by:
Priority: undecided Milestone: 1.5
Component: unfiled Version: 1.4.3
Keywords: Cc: Rick Waldron, snover, ajpiano
Blocked by: Blocking:

Description

So jQuery.is(selector) suppose to return 'true' or 'false' but it doesn't seem to work with with the pseudo selectors :first & :last.

In a list of 3 items:

('li:first').is('li:last') //true, wrong, Bug?
('li:eq(0)').is('li:eq(2)')  //false, correct

http://jsfiddle.net/882Tc/



Tried it in a bunch of previous version and got the same result.

I'm not sure if I'm doing something wrong or I'm not understanding :first correctly... but it seems to me that :first & :last doesn't work correctly with jQuery.is()

Thanks You

Change History (15)

comment:1 Changed 9 years ago by addyosmani

Keywords: is first last equal added
Status: newopen

I've thoroughly evaluated this here http://jsfiddle.net/882Tc/10/ and it would appear that if you directly compare any item as follows $('elem:first').is('elem:last') is will return a result 'true'. If however you cache the variables beforehand and do an $x.is($y) comparison that returns the correct result of false. Internally both :first,:last and .is are fairly simple in logic and it could be the case that we require additional logic to go in there to handle these cases. As mentioned, using caching allows you to get this working. Leaving open for further review.

comment:2 Changed 9 years ago by qvprof@…

Hey addyosmani,

Thanks for checking the bug.

Not sure if it's related but when you cache an $x2 then do a $x.is($x2) returns false. Which it would seem should return true...

http://jsfiddle.net/882Tc/11/

comment:3 Changed 9 years ago by jitter

Partly this seems to be a misconception on how is( selector ) works.

One thing beforehand: is( selector ) only works with strings. From http://api.jquery.com/is/

selector A string containing a selector expression to match elements against.

So you can take out of the picture all the tests where you tried to pass in an object as is will always return false in such a case.

Some of the remaining sample tests are false negatives to me. The thing to keep in mind is that the is-function uses the matched elements as context not the document.

live test case with explanations why I think most use cases work right.

But there seems to be some kind of a bug. If you look at the last two tests

<ul class="test" style="display:none">
    <li>one</li>
    <li>two</li>
    <li>three</li>
</ul>

//true -> ERROR
$( 'li:first' ).is( '.test li:last' )

//true -> ERROR
$( 'li:first' ).filter( '.test li:last' ).length > 0

Both should return false in my opinion as there is only a li in the set of matched elements which neither has the class test assigned nor has it any li children.

If you switch from 1.4.3 to 1.4.2 another strange thing happens.

The filter test returns false correctly but the is test still returns true. So we have both a regression in filter and a case where

$( sel ).is( secondsel ) != $( sel ).filter( secondsel ).length > 0

which shouldn't happen.

More discussion needed. Does someone follow my view on this?

Last edited 9 years ago by jitter (previous) (diff)

comment:4 Changed 9 years ago by qvtree

Hey jitter, thanks for testing.

I didn't realize that jQuery.is filters based on it's context. I always thought it was global.

I ran a few more test cases and think that it actually isn't a bug after all. But what do you guys think?

Seems the reason why:

//Expected False but its True?
log($( 'li:first' ).is( '.test li:last' ), false);


Might be because li:first IS below .test AND is :last in it's context (of only one list item)

So perhaps the jQuery.is documentation just needs to make it clearer that it's contextual and explains that for hierarchy searches, it goes outside it's context, for pseudo searches it stays within it's context.

But perhaps it wasn't meant to work like that.

comment:5 Changed 9 years ago by jitter

Hmm now I am somewhat confused. After looking at this again I ask myself how/why earlier today I came to the conclusion that jQuery.is( selector ) is using the matched elements as context and not document.

Right now I came to the opposite conclusion which would match the original statements of qvprof and addyosmani and make the better part of my previous comment invalid (that's why I did stroke it out). Maybe I was looking at something else...wtf?

Can someone double check this?

So most of my previous comment can be forgotten because it is wrong (or is it). The only thing which stands is my remark about the tests which pass in an object.

I made some further tests test case trying to trim down where the bug occurs. And especially this version of the bug I found doesn't make any sense at all

<ul class="test" style="display:none">
    <li>one</li>
    <li>two</li>
    <li>three</li>
</ul>
<ul style="display:none">
    <li><p>asd</p></li>
</ul>

$( 'li:first' ).is( 'ul:last p' ) //why does this yield true
$( 'li:first' ).is( 'ul p:last' ) //why does this yield true

It looks like there are some bigger problems with some pseudo selectors (:first, :last, :eq(), :even, :odd and maybe others too) they don't occur always. Sometimes it needs a special formed selector to trigger the bug (see also my comments in the testcase)

comment:6 Changed 9 years ago by hoganlong@…

It seems to me that the real problem is the word "is". If this function was called "has" all the test cases would make sense. There would be no bug.

Right now the functionality is a "has", the test is performed in the context of the result. The first item of a list is also the last item of itself.

comment:7 Changed 9 years ago by addyosmani

Cc: Rick Waldron snover ajpiano added

I wanted to close this as a ticket requiring improved documentation on the expected/actual behavior of 'is' however, I think a team review of Jitter's comments are worth taking into account. What is the general feeling on the issues presented here?

comment:8 Changed 9 years ago by awellis13

I still think there's a bug related to is(':first'). Isn't :first an alias of :first-child? The following does not work:

$('div', $(this)).each(function(){
   if ($(this).is(':first')) {
       $(this).addClass('current');
   } else {
       $(this).hide();
   }
});

However, this does work:

$('div', $(this)).each(function(){
   if ($(this).is(':first-child')) {
       $(this).addClass('current');
   } else {
       $(this).hide();
   }
});

comment:9 Changed 9 years ago by ajpiano

Keywords: needsdoc added

:first is not an alias for :first-child.

Just so everyone is clear what the bug is here, it seems to be thatthat jQuery's custom selectors that are used for checking elements against their position in the jQuery object (:first, :last, :eq, etc) are failing when used in .is() against another stringy selector. These selectors are NOT part of QSA, but are obviously useful for a variety of reasons.

Part of me isn't even sure there is something that is worth fixing here, it might be more of a docs issue. While there are lots of valid uses for something that checks DOM position, like .is(":last-child"), I'm unable to think of a viable use case for testing elements against their relative position in an arbitrary selection of elements, which is what .is(":last") does.

comment:10 in reply to:  9 Changed 9 years ago by awellis13

Replying to ajpiano:

:first is not an alias for :first-child.

Just so everyone is clear what the bug is here, it seems to be thatthat jQuery's custom selectors that are used for checking elements against their position in the jQuery object (:first, :last, :eq, etc) are failing when used in .is() against another stringy selector. These selectors are NOT part of QSA, but are obviously useful for a variety of reasons.

Part of me isn't even sure there is something that is worth fixing here, it might be more of a docs issue. While there are lots of valid uses for something that checks DOM position, like .is(":last-child"), I'm unable to think of a viable use case for testing elements against their relative position in an arbitrary selection of elements, which is what .is(":last") does.

I guess you're right. It is just confusing reading this in the docs for :first:

The :first pseudo-class is equivalent to :eq(0). It could also be written
as :lt(1). While this matches only a single element, :first-child can match
more than one: One for each parent.

It just makes it sound like :first could be used like :first-child for that kind of scenario. I'll admit, the jQuery documentation could use a major overhaul; especially the plugin authoring documentation.

comment:11 Changed 9 years ago by ajpiano

There are a lot of cases where :first and :first-child will act the same, and a lot of places where they won't. .is() is clearly one of these cases.

WRT to the plugin authoring docs, if you would like to discuss that further, we'd welcome you to hop onto IRC (freenode - #jquery-dev) or start a forum thread on your ideas. The plugin authoring guide was just redone a few months ago, and seems to me to be in reasonable shape. In any case, it's outside the scope of this ticket so I'mma stop typing now :)

comment:12 Changed 9 years ago by jitter

Keywords: needsdocs added; needsdoc removed

comment:13 Changed 9 years ago by dmethvin

Resolution: worksforme
Status: openclosed

jitter, you were right about the context being the current set. So...

$("li:first").is("li:last")

selects the first li in the document and compares it to the the selector. The is() method is basically:

jQuery.filter( selector, this ).length > 0

Where selector=="li:last" and this is a jQuery set containing the element selected by li:first.

So, does a single li element match the selector li:last? Yes!

Note that this also explains the $('li:eq(0)').is('li:eq(2)') case being false, a single li element will never match the selector li:eq(2).

I'm going to close this ticket but leave the needsdocs so we can review and clarify.

comment:14 Changed 9 years ago by jmm

I ran into the same issue and found this report. If the current behavior of is() -- working only in the context of the current set -- is the intended behavior, then I'd say the documentation definitely needs work, because it's not at all clear that it will behave that way.

I don't know what implementation issues it would present, but it seems like is() would be more useful if it wasn't limited to the context of the current set and could use an arbitrary selector.

I ran into this while writing a keydown handler for elements in a modal dialog. While it's displayed I want to restrict the keyboard focus to elements in the dialog. So for example, when pressing Tab on the last focusable element in the dialog (or Shift+Tab on the first) I want to prevent focus from being given to an element outside of the dialog. I ran into this issue when I tried to accomplish that using is() like this:

$( this ).is( "#dialog input:first" )

(Where this is any INPUT element in the dialog.)

comment:15 Changed 9 years ago by dmethvin

Keywords: needsdocs is first last equal removed
Note: See TracTickets for help on using tickets.