Bug Tracker

Ticket #8380 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

$(':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:
Blocking: Blocked by:

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

comment:1 Changed 3 years ago by mathias

For the record, when this is fixed, the docs should probably be updated as well:  http://api.jquery.com/text-selector/

comment:2 Changed 3 years ago by mathias

comment:3 Changed 3 years ago by addyosmani

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

Thanks. The pull request looks fairly decent - CC'ing snover for further review.

comment:4 Changed 3 years ago by dmethvin

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 3 years ago by dmifedorenko@…

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 3 years ago by mathias

@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 3 years ago by dmethvin

  • Priority changed from low to blocker
  • Milestone changed from 1.next to 1.5.2

comment:8 Changed 3 years ago by dmethvin

#8163 is a duplicate of this ticket.

comment:9 Changed 3 years ago by gnarf

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 };

 http://jsfiddle.net/CpvgM/1/

comment:10 Changed 3 years ago by snover

  • Cc snover removed

comment:11 Changed 3 years ago by anonymous

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 3 years ago by john

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

comment:13 Changed 3 years ago by dmethvin

#8565 is a duplicate of this ticket.

comment:14 Changed 3 years ago by john

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

Landed mathias' pull request.

comment:15 Changed 3 years ago by danheberden

  • Keywords needsdocs added

comment:16 Changed 3 years ago by danheberden

Per matjas: [8:29 AM] <matjas> now that #8380 is fixed, the text on  http://api.jquery.com/text-selector/ needs some tweaking

comment:17 Changed 3 years ago by addyosmani

#8583 is a duplicate of this ticket.

comment:18 Changed 3 years ago by kswedberg

  • Keywords needsdocs removed

updated documentation

Note: See TracTickets for help on using tickets.