#8380 closed bug (fixed)
$(':text') should match <input> since `type=text` is the implied default
Reported by: | mathias | Owned by: | john |
---|---|---|---|
Priority: | blocker | Milestone: | 1.5.2 |
Component: | selector | Version: | 1.5.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
Test case: http://jsfiddle.net/mathias/CpvgM/
Currently, $(':text') only matches inputs with type=text explicitly set. It should also match <input>
since type=text is the implied default in all browsers (as per the HTML spec).
Change History (18)
comment:1 Changed 12 years ago by
comment:3 Changed 12 years ago by
Cc: | snover added |
---|---|
Component: | unfiled → selector |
Priority: | undecided → low |
Status: | new → open |
Thanks. The pull request looks fairly decent - CC'ing snover for further review.
comment:4 Changed 12 years ago by
We talked about this on -dev today, it seems like a good idea. However we should discourage <input />
in markup since it does not match qSA for input[type=text]
-- the attribute is missing, only the property is "text".
comment:5 Changed 12 years ago by
It is critical regression for my projects, I did not set type attribute for text input _several years_.
So now I need edit and test tons of HTML/XSL/PHP/JS templates to upgrade to jQuery 1.5.1?
comment:6 Changed 12 years ago by
@dmifedorenko: Actually, no. $(':text')
should match <input>
and it has always worked that way before jQuery 1.5.1 was released. The regression you’re talking about is in 1.5.1, and this patch will fix it.
comment:7 Changed 12 years ago by
Milestone: | 1.next → 1.5.2 |
---|---|
Priority: | low → blocker |
comment:9 Changed 12 years ago by
I probably introduced this bug in 1.5.1 - https://github.com/gnarf37/sizzle/commit/0048e13aa17bd805374784f7d6e6f6f0a0140fe7 -- Before that commit, :text
just did "text" === elem.type -- we could potentially revert that function (not the whole commit since :text
is a Sizzle only "magic" selector it can do whatever we need it to.
If you want to override this in 1.5.1 for your own regression purposes its a one line fix:
jQuery.find.selectors.filters['text'] = function(elem) { return "text" === elem.type };
comment:10 Changed 12 years ago by
Cc: | snover removed |
---|
comment:11 Changed 12 years ago by
Wouldn't this work? (to replace line 619 from https://github.com/gnarf37/sizzle/commit/0048e13aa17bd805374784f7d6e6f6f0a0140fe7)
return "text" === (elem.getAttribute( 'type' ) || "text");
As it will return the value of type, and if this is null, it will return the default: "text".
comment:12 Changed 12 years ago by
Owner: | set to john |
---|---|
Status: | open → assigned |
comment:14 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Landed mathias' pull request.
comment:15 Changed 12 years ago by
Keywords: | needsdocs added |
---|
comment:16 Changed 12 years ago by
Per matjas: [8:29 AM] <matjas> now that #8380 is fixed, the text on http://api.jquery.com/text-selector/ needs some tweaking
For the record, when this is fixed, the docs should probably be updated as well: http://api.jquery.com/text-selector/