#8039 closed bug (fixed)

Opened January 24, 2011 10:35AM UTC

Closed February 17, 2011 03:51PM UTC

Last modified March 09, 2012 07:12AM UTC

Selectors with new HTML(5) input types don’t work in <= IE7

Reported by:	mathias	Owned by:	gnarf
Priority:	low	Milestone:	1.5.1
Component:	selector	Version:	1.5
Keywords:		Cc:
Blocked by:		Blocking:

Description

HTML:

<input type="search">

jQuery:

$('input[type="search"]').length; // should be 1, but is 0 in <= IE7
$('input[type="text"]').length; // should be 0, but is 1 in <= IE7
$('input').attr('type'); // should be "search", but is "text" in <= IE7
document.getElementsByTagName('input')[0].getAttribute('type'); // "search" (works everywhere)

It seems jQuery is using elem.type internally instead of elem.getAttribute('type'). In this case, there’s an important difference.

I’ve created a jsFiddle to illustrate the bug: http://jsfiddle.net/mathias/XzGcW/ Note that it uses alert() so it can be debugged in a clean IE without any plugins installed.

P.S. This does not fix the bug, but I think that in src/selector.js, around line 613, this should be rewritten to elem.getAttribute('type') to prevent similar bugs with :text:

text: function( elem ) {
	return "text" === elem.type;
}

Attachments (0)

Change History (12)

Changed January 24, 2011 10:52AM UTC by mathias comment:1

_comment0:	In `src/attributes.js`, near the end of `attr`, \ \ {{{ \ return elem[ name ]; \ }}} \ \ …should be: \ \ {{{ \ return elem.getAttribute(name); \ }}} \ \ This fixes the `.attr()` issue, but not the selector bug. → 1295866413530068
_comment1:	In `src/attributes.js`, near the end of `attr` (around line 346): \ \ {{{ \ return elem[ name ]; \ }}} \ \ …should probably be: \ \ {{{ \ return elem.getAttribute(name); \ }}} \ \ This fixes the `.attr()` issue, but not the selector bug. → 1295893006425129

In src/attributes.js, near the end of attr (around line 346):

return elem[ name ];

…should probably be:

return elem.getAttribute(name);

This fixes the .attr() issue, but not the selector bug.

Edit: Of course, this change causes errors with className etc, see comment 3 by rwaldron.

Changed January 24, 2011 11:37AM UTC by gnarf comment:2

_comment0:	One fix is to add a special case for the 'type' ATTR check that will force .getAttribute: \ {{{ \ Sizzle.selectors.attrHandle['type'] = function ( elem ) { \ return elem.getAttribute( 'type' ); \ } \ }}} \ \ ( seen on http://jsfiddle.net/gnarf/XzGcW/1/ ) \ \ Also, I'm not really sure why the default behavior in the attr function around ~790 of sizzle.js prefers to use {{{ elem[ name ] }}} over {{{ elem.getAttribute( name ) }}} but switching this to prefer {{{ elem.getAttribute }}} may fix the need to have {{{ attrHandle }}} at all since both the current one (href) and the proposed one for type just {{{ return .getAttribute() }}} → 1295869389888913
_comment1:	One fix is to add a special case for the 'type' ATTR check that will force .getAttribute: \ {{{ \ Sizzle.selectors.attrHandle['type'] = function ( elem ) { \ return elem.getAttribute( 'type' ); \ } \ }}} \ \ ( change {{{ Sizzle }}} to {{{ jQuery.find }}} and you can monkey patch this bug on your own page: seen on http://jsfiddle.net/gnarf/XzGcW/1/ ) \ \ Also, I'm not really sure why the default behavior in the attr function around ~790 of sizzle.js prefers to use {{{ elem[ name ] }}} over {{{ elem.getAttribute( name ) }}} but switching this to prefer {{{ elem.getAttribute }}} may fix the need to have {{{ attrHandle }}} at all since both the current one (href) and the proposed one for type just {{{ return .getAttribute() }}} → 1295869441417706

_comment0:

One fix is to add a special case for the 'type' ATTR check that will force .getAttribute: \ {{{ \ Sizzle.selectors.attrHandle['type'] = function ( elem ) { \ return elem.getAttribute( 'type' ); \ } \ }}} \ \ ( seen on http://jsfiddle.net/gnarf/XzGcW/1/ ) \ \ Also, I'm not really sure why the default behavior in the attr function around ~790 of sizzle.js prefers to use {{{ elem[ name ] }}} over {{{ elem.getAttribute( name ) }}} but switching this to prefer {{{ elem.getAttribute }}} may fix the need to have {{{ attrHandle }}} at all since both the current one (href) and the proposed one for type just {{{ return .getAttribute() }}} → 1295869389888913

_comment1:

One fix is to add a special case for the 'type' ATTR check that will force .getAttribute: \ {{{ \ Sizzle.selectors.attrHandle['type'] = function ( elem ) { \ return elem.getAttribute( 'type' ); \ } \ }}} \ \ ( change {{{ Sizzle }}} to {{{ jQuery.find }}} and you can monkey patch this bug on your own page: seen on http://jsfiddle.net/gnarf/XzGcW/1/ ) \ \ Also, I'm not really sure why the default behavior in the attr function around ~790 of sizzle.js prefers to use {{{ elem[ name ] }}} over {{{ elem.getAttribute( name ) }}} but switching this to prefer {{{ elem.getAttribute }}} may fix the need to have {{{ attrHandle }}} at all since both the current one (href) and the proposed one for type just {{{ return .getAttribute() }}} → 1295869441417706

One fix is to add a special case for the 'type' ATTR check that will force .getAttribute:

 
Sizzle.selectors.attrHandle['type'] = function ( elem ) { 
    return elem.getAttribute( 'type' ); 
}

( change

 Sizzle

 jQuery.find

and you can monkey patch this bug on your own page: seen on http://jsfiddle.net/gnarf/XzGcW/2/ )

Also, I'm not really sure why the default behavior in the attr function around ~790 of sizzle.js prefers to use

 elem[ name ]

over

 elem.getAttribute( name )

but switching this to prefer

 elem.getAttribute

may fix the need to have

 attrHandle

at all since both the current one (href) and the proposed one for type just

 return .getAttribute()

Changed January 24, 2011 02:24PM UTC by rwaldron comment:3

component:	unfiled → attributes
owner:	→ mathias
status:	new → pending

I just applied this change and failed the following...

attributes: attr(String) (5, 14, 19)

2. Check for defaultValue attribute

10. Check for class attribute

17. Check for selectedIndex attribute

18. Died on test #18: Cannot call method 'toUpperCase' of null

attributes: attr(String, Object) (1, 29, 30)

3. Remove name attribute

attributes: addClass(String) (3, 2, 5)

3. Make sure there's no extra whitespace.

4. Make sure there's no extra whitespace.*

5. Make sure there isn't too much trimming.

attributes: addClass(Function) (3, 2, 5)

3. Make sure there's no extra whitespace.

4. Make sure there's no extra whitespace. *

5. Make sure there isn't too much trimming.

yes, both named this

Changed January 24, 2011 06:23PM UTC by rwaldron comment:4

component:	attributes → selector
priority:	undecided → low
status:	pending → open

Changed January 24, 2011 07:17PM UTC by gnarf comment:5

Potential Fix: https://github.com/jeresig/sizzle/pull/48

Changed January 24, 2011 07:50PM UTC by mathias comment:6

I can confirm that gnarf’s patch successfully fixes the problem, while passing all unit tests in all supported browsers, including IE7 and IE6.

Changed January 24, 2011 11:31PM UTC by jitter comment:7

owner:	mathias → gnarf
status:	open → assigned
version:	git → 1.5rc1

Changed January 28, 2011 01:38PM UTC by jitter comment:8

milestone:	1.next → 1.5.1

Changed February 03, 2011 06:08PM UTC by jitter comment:9

version:	1.5rc1 → 1.5

Changed February 03, 2011 06:10PM UTC by jitter comment:10

#8164 is a duplicate of this ticket.

Changed February 03, 2011 06:16PM UTC by jitter comment:11

blocking:	→ 8163

Changed February 17, 2011 03:51PM UTC by jitter comment:12

resolution:	→ fixed
status:	assigned → closed

Fixed https://github.com/jeresig/sizzle/commit/ab88b457fe922f74a1777e558081641803313e17

Side navigation