Skip to main content

Bug Tracker

Side navigation

#13011 closed bug (fixed)

Opened December 06, 2012 03:29PM UTC

Closed December 12, 2012 04:16AM UTC

Setting type attribute on an input doesn't work as intended

Reported by: anonymous Owned by: dmethvin
Priority: low Milestone: 1.9
Component: attributes Version: 1.8.3
Keywords: Cc:
Blocked by: Blocking:
Description

Testcase: http://jsfiddle.net/yeqcf/1/

Using the following example code:

        <input id="one" type="email" />
        <input id="two" />
        $('#one').attr('type', 'text');
        $('#two').attr('type', 'text');

doesn't produce the results jQuery developers seem to have intended in my own testing.

Looking at the jQuery source, on line 319 of attributes.js we have:

if ( hooks && notxml && "set" in hooks && (ret = hooks.set( elem, value, name )) !== undefined ) {
	return ret;
}

In most browsers, this runs hooks.set() which throws jQuery.error() and then returns "ret". However in IE it seems to be continuing (i.e. hooks.set() is run, but the statement evaluates false so the attribute is set regardless.

This is problematic for two reasons:

1. browsers that can change the type attribute are throwing an error, while IE (which can't) is continuing.

2. IE can *set* the type attribute if it hasn't been already (in example #two above) so this should work everywhere (and throw no errors in either browsers).

Notes:

Using jQuery 1.8.3 - tested in Firefox 17.0.1, Opera 12.11, Chrome 23.0.1271.95 m and IE9 applying IE8 + IE7 modes via dev tools.

Attachments (0)
Change History (10)

Changed December 06, 2012 04:03PM UTC by dmethvin comment:1

I'm not seeing the behavior you describe, using jquery-git.js on both real IE8 and on IE9 selecting IE7, IE8, and IE9 modes.

http://jsfiddle.net/yeqcf/2/show/

The justification for making setting type an error was that web devs don't do their main development in oldIE and therefore would create code that didn't run on oldIE but not realize it until late in their dev process. Throwing an error in all browsers let people fix it early, but at the cost of preventing good browsers from doing something legit. I believe this is the only place in all of jQuery where we made this sort of decision.

https://github.com/jquery/jquery/blob/26bf8dd56bfbef54a07cbba519485dcd59add839/src/attributes.js#L369

I think we should just remove the check for "type" in both 1.9 and 2.0. The restriction isn't needed in 2.0 for sure and I think we should absolutely remove it there, but I'm concerned that this will be a "OMG THERE IS A DIFFERENCE BETWEEN 1.9 AND 2.0" complaint if we only remove it in 2.0.

Thoughts?

Changed December 06, 2012 04:06PM UTC by scottgonzalez comment:2

Remove it in both.

Changed December 06, 2012 11:04PM UTC by timmywil comment:3

The point is to provide cross-browser consistency. If I want to support old IE AND want to change the type of an input, jQuery should continue to let me know that it aint gonna work. In 2.0, it will work across all supported browsers. Therefore, no need to show the error. I think users will understand this. We'll probably get a few tix complaining that it can't be done in 1.9. But...well...it can't.

Changed December 06, 2012 11:04PM UTC by timmywil comment:4

component: unfiledattributes
milestone: None1.9
priority: undecidedlow
status: newopen

Changed December 06, 2012 11:16PM UTC by scottgonzalez comment:5

I disagree, this is like setting any other property or style that a specific browser doesn't understand. The fact that it throws an error in some browsers doesn't mean we should force that behavior in all browsers. We patch over these kinds of problems where possible, but this is the only place I can think of where we do the opposite and force the restriction/error everywhere.

Changed December 07, 2012 01:19AM UTC by dmethvin comment:6

Yeah, the uniqueness of this case is what has me thinking we should remove the thrown error. Even oldIE lets you set the property once if it's created with createElement(), but we throw when any browser tries to write it.

Changed December 07, 2012 03:30PM UTC by timmywil comment:7

_comment0: ''The fact that it throws an error in some browsers doesn't mean we should force that behavior in all browsers.'' \ yes it does \ \ "We patch over these kinds of problems where possible, but this is the only place I can think of where we do the opposite and force the restriction/error everywhere." \ \ We can't patch over this case. The only alternative here is to allow it when we shouldn't. \ \ @dmethvin: actually, we account for the `createElement` case.1354894335503791

''The fact that it throws an error in some browsers doesn't mean we should force that behavior in all browsers.''

yes it does

''We patch over these kinds of problems where possible, but this is the only place I can think of where we do the opposite and force the restriction/error everywhere.''

We can't patch over this case. The only alternative here is to allow it when we shouldn't.

@dmethvin: actually, we account for the createElement case.

Changed December 07, 2012 03:51PM UTC by dmethvin comment:8

Only oldIE thinks that setting the type a second time should be an error. Other browsers allow it. So if we were to continue to throw the error in 1.9, should we continue the embargo into 2.0 and throw the error there? Why would we want to artificially limit something that all the browsers can do? If we allow 1.9 and 2.0 to do different things in this case, I see that as a bigger problem than only throwing an error in oldIE, something that can be worked around or avoided in user code.

Changed December 10, 2012 05:18PM UTC by dmethvin comment:9

owner: → dmethvin
status: openassigned

Changed December 12, 2012 04:16AM UTC by Dave Methvin comment:10

resolution: → fixed
status: assignedclosed

Fix #13011. Let 'type' attribute be set if the browser allows.

Changeset: aad235b3251494afe71fd5bb6031e11965af9bdb