Bug Tracker

Opened 10 years ago

Closed 10 years ago

#13011 closed bug (fixed)

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.

Change History (10)

comment:1 Changed 10 years ago by dmethvin

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?

comment:2 Changed 10 years ago by scottgonzalez

Remove it in both.

comment:3 Changed 10 years ago by Timmy Willison

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.

comment:4 Changed 10 years ago by Timmy Willison

Component: unfiledattributes
Milestone: None1.9
Priority: undecidedlow
Status: newopen

comment:5 Changed 10 years ago by scottgonzalez

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.

comment:6 Changed 10 years ago by dmethvin

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.

comment:7 Changed 10 years ago by Timmy Willison

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.

Version 0, edited 10 years ago by Timmy Willison (next)

comment:8 Changed 10 years ago by dmethvin

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.

comment:9 Changed 10 years ago by dmethvin

Owner: set to dmethvin
Status: openassigned

comment:10 Changed 10 years ago by Dave Methvin

Resolution: fixed
Status: assignedclosed

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

Changeset: aad235b3251494afe71fd5bb6031e11965af9bdb

Note: See TracTickets for help on using tickets.