Bug Tracker

Ticket #11115 (closed bug: fixed)

Opened 3 years ago

Last modified 23 months ago

".is()" and ".filter()" disagree on attribute selector "[checked]"

Reported by: Pointy Owned by: gibson042
Priority: blocker Milestone: 1.9
Component: selector Version: 1.7.1
Keywords: 1.9-discuss Cc:
Blocking: Blocked by: #12600

Description

The selector "[checked]" is treated differently by ".is()" and ".filter()", in a strange and mysterious way when the "checked" property (not attribute) is set; that is, on checkbox inputs that in the DOM lack the "checked" attribute but which have been checked by user action (or possibly by explicit setting of the "checked" property). If you've got a jQuery object matching one DOM node (and just one), then ".is()" and ".filter()" are in agreement. That is, if a checkbox is checked only after the DOM is built, then both ".is()" and ".filter()" agree that the selector "[checked]" should not match the node.

However, when the jQuery object selects more than one checkbox, then the behavior of ".filter()" changes. Though each checkbox, if tested individually with ".is('[checked]')" would appear to not match, a call to ".filter('checked')" will nevertheless include them in the result if they've been dynamically updated.

Here is a jsfiddle to explore the issue:  http://jsfiddle.net/mgfxe/2/

Now I realize that "[checked]" is not really correct or useful or anything, but the behavior does seem inconsistent; I would imagine that any node that makes it through a selector-based ".filter()" should be guaranteed to pass a ".is()" test for the same selector. The funny thing is that the implementation boils down to calls to "Sizzle.matches()" or "Sizzle.matchesSelector()" (I think) and both of those are nearly identical.

Change History

comment:1 Changed 3 years ago by dcherman

Here's my take:

In browsers that support it, Sizzle.matchesSelector is redefined to

html.matchesSelector
html.mozMatchesSelector html.webkitMatchesSelector html.msMatchesSelector;

When one of the checkboxes is run through elem.webkitMatchesSelector( "[checked]" ), the result is false.

When the code is run through Sizzle's ATTR filter, it hits  https://github.com/jquery/sizzle/blob/423f35af8bf43f3f07bb17284e21608f08372c22/sizzle.js#L860-861 which is actually checking the property, so it returns true.

If your jQuery object has more than one element, it'll follow the "normal" Sizzle path which happens to check the elements' property. With a single element, it follows the .matchesSelector path which has been shown to return false.

Bit longwinded, but that was interesting :)

A simple workaround might be to use the :checked pseudo selector which always checks the property rather than the attribute.

Last edited 3 years ago by dcherman (previous) (diff)

comment:2 Changed 3 years ago by Pointy

Totally agree that this is an academic issue, but it's pretty hard to think of it as anything but incorrect behavior.

comment:3 Changed 3 years ago by timmywil

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to selector

Removing the boolhook would fix this issue, but I don't think we can remove it for a while. Nevertheless, the ideal way to check if a checkbox is checked in a selector is with  http://api.jquery.com/checked-selector/.

comment:4 Changed 2 years ago by dmethvin

#11667 is a duplicate of this ticket.

comment:5 Changed 2 years ago by gibson042

  • Keywords 1.9-discuss added

comment:6 Changed 2 years ago by dmethvin

So are we going to attempt to remove boolHook in 1.9? If not I think we have to assume boolHook will be there forever and this will need to be fixed some other way or closed wontfix.

comment:7 Changed 2 years ago by gibson042

boolHook has been on notice since 1.6, right? I think it's time.

comment:8 Changed 2 years ago by mikesherov

+1, I'm not for removing the boolHook, but I am for making is and filter consistent.

comment:9 Changed 2 years ago by timmywil

#12795 is a duplicate of this ticket.

comment:10 Changed 2 years ago by mikesherov

  • Milestone changed from None to 1.9

comment:11 Changed 2 years ago by timmywil

  • Owner set to timmywil
  • Status changed from open to assigned

comment:12 Changed 2 years ago by gibson042

#12830 is a duplicate of this ticket.

comment:13 follow-up: ↓ 14 Changed 2 years ago by Allen Schmidt <cobrasoft@…>

I did some investigating, and I'm guessing this is an issue with querySelectorAll(). It seems that "[selected]" and "[checked]" return elements that initially had this attribute on page load. This is only an issue when used like "option[selected]" since jQuery selects that using querySelectorAll() when available. When using filter("[selected]"), it is not using querySelectorAll(), and therefore returns the correct results.

comment:14 in reply to: ↑ 13 Changed 2 years ago by gibson042

Replying to Allen Schmidt <cobrasoft@…>:

I did some investigating, and I'm guessing this is an issue with querySelectorAll(). It seems that "[selected]" and "[checked]" return elements that initially had this attribute on page load. This is only an issue when used like "option[selected]" since jQuery selects that using querySelectorAll() when available. When using filter("[selected]"), it is not using querySelectorAll(), and therefore returns the correct results.

You've got it exactly backwards... [selected] is an attribute selector, and jQuery incorrectly treats elements having true analogous properties as matching it, while querySelectorAll correctly ignores them.

comment:15 Changed 2 years ago by gibson042

  • Blocked by 12600 added

(In #12600) The expectations in the comments of that fiddle are erroneous... select[value=…] shouldn't match anything, because most select elements—and those in particular—don't have attributes named "value".

This ticket is the attrHooks equivalent of #11115: native Sizzle always gets it right, jQuery only does so when jQuery.filter calls a qSA-backed matchesSelector in the single-element case:  http://jsfiddle.net/Mj23Q/43/

Last edited 2 years ago by gibson042 (previous) (diff)

comment:16 Changed 23 months ago by gibson042

  • Priority changed from low to blocker

The hook should be moved to  https://github.com/jquery/jquery-compat/.

comment:17 Changed 23 months ago by gibson042

  • Owner changed from timmywil to gibson042

comment:18 Changed 23 months ago by Richard Gibson

  • Status changed from assigned to closed
  • Resolution set to fixed

Fix #11115: Normalize boolean attributes/properties. Close gh-1066.

Changeset: a763ae72775f69509d0a8a6b50702bcf2c7e6fbf

Note: See TracTickets for help on using tickets.