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 comment:1
Changed December 06, 2012 04:06PM UTC by comment:2
Remove it in both.
Changed December 06, 2012 11:04PM UTC by 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 comment:4
| component: | unfiled → attributes |
|---|---|
| milestone: | None → 1.9 |
| priority: | undecided → low |
| status: | new → open |
Changed December 06, 2012 11:16PM UTC by 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 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 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 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 comment:9
| owner: | → dmethvin |
|---|---|
| status: | open → assigned |
Changed December 12, 2012 04:16AM UTC by comment:10
| resolution: | → fixed |
|---|---|
| status: | assigned → closed |
Fix #13011. Let 'type' attribute be set if the browser allows.
Changeset: aad235b3251494afe71fd5bb6031e11965af9bdb
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
typean 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?