Fix #6562, tighten up the special code for form objects, add name attrHook for IE6/7, and don't check for undefined with getting hook'd attr

src/attributes.js

@@ -275,7 +275,10 @@ jQuery.extend({
 		offset: true
 	},
 	
-	attrFix: {},
+	attrFix: {
+		// Always normalize to ensure hook usage
+		tabindex: "tabIndex"
+	},
 	
 	attr: function( elem, name, value, pass ) {
 		var nType = elem.nodeType;
@@ -290,7 +293,8 @@ jQuery.extend({
 		}
 		
 		var ret, hooks,
-			notxml = nType !== 1 || !jQuery.isXMLDoc( elem );
+			notxml = nType !== 1 || !jQuery.isXMLDoc( elem ),
+			isFormObjects = !jQuery.support.getSetAttribute && elem.nodeName === "FORM";
 		
 		// Normalize the name if needed
 		name = notxml && jQuery.attrFix[ name ] || name;
@@ -307,17 +311,30 @@ jQuery.extend({
 				return undefined;
 
 			} else {
-				elem.setAttribute( name, value );
+				
+				// Check form objects in IE (multiple bugs related)
+				if ( isFormObjects ) {
+					elem.getAttributeNode( name ).nodeValue = value;
+				} else {
+					elem.setAttribute( name, value );
+				}
 				return value;
 			}
 
 		} else {
 
-			if ( hooks && "get" in hooks && notxml && ((ret = hooks.get( elem )) !== undefined || name === "tabIndex") ) {
+			if ( hooks && "get" in hooks && notxml ) {
-				return ret;
+				return hooks.get( elem );
 
 			} else {
-				ret = elem.getAttribute( name );
+				
+				// Check form objects in IE (multiple bugs related)
+				if ( isFormObjects ) {
+					// Returns undefined for empty string, which is the blank nodeValue in IE
+					ret = elem.getAttributeNode( name ).nodeValue || undefined;
+				} else {
+					ret = elem.getAttribute( name );
+				}
 
 				// Non-existent attributes return null, we normalize to undefined
 				// Instead of checking for null, we check for typeof object to catch inputs in IE6/7. Bug #7472
@@ -334,13 +351,9 @@ jQuery.extend({
 		if ( jQuery.support.getSetAttribute ) {
 			elem.removeAttribute( name );
 		} else {
-			// set property to null if getSetAttribute not supported (IE6-7)
+			// use DOM level 1 if getSetAttribute not supported (IE6-7)
-			// setting className to null makes the class "null"
+			elem.setAttribute( name, "" ); // Set to default empty string
-			if ( name === "className" ) {
+			elem.removeAttributeNode( elem.getAttributeNode( name ) );
-				elem[ name ] = "";
-			} else {
-				elem.setAttribute( name, null );
-			}
 		}
 	},
 
@@ -352,6 +365,19 @@ jQuery.extend({
 					jQuery.error( "type property can't be changed" );
 				}
 			}
+		},
+		tabIndex: {
+			get: function( elem ) {
+				// elem.tabIndex doesn't always return the correct value when it hasn't been explicitly set
+				// http://fluidproject.org/blog/2008/01/09/getting-setting-and-removing-tabindex-values-with-javascript/
+				var attributeNode = elem.getAttributeNode("tabIndex");
+
+				return attributeNode && attributeNode.specified ?
+					parseInt( attributeNode.value, 10 ) :
+					rfocusable.test( elem.nodeName ) || rclickable.test( elem.nodeName ) && elem.href ?
+						0 :
+						undefined;
+			}
 		}
 	},
 	
@@ -404,23 +430,15 @@ if ( !jQuery.support.getSetAttribute ) {
 		cellspacing: "cellSpacing",
 		rowspan: "rowSpan",
 		colspan: "colSpan",
-		tabindex: "tabIndex",
 		usemap: "useMap",
 		frameborder: "frameBorder"
 	});
 	
-	// Action attribute in ie6/7 returns form objects
+	// Name attribute will not get removed in browsers that do not support getSetAttribute
-	jQuery.attrHooks.action = {
+	// Return undefined on empty string or null
-		get: function( elem ) {
+	jQuery.attrHooks.name = {
-			return elem.nodeName === "FORM" ?
+		get: function( elem, value ) {
-				elem.getAttributeNode("action").nodeValue :
+			return elem.getAttributeNode("name").nodeValue || undefined;
-				elem.getAttribute("action");
-		},
-		set: function( elem, value ) {
-			elem.nodeName === "FORM" ?
-				elem.getAttributeNode("action").nodeValue = value :
-				elem.setAttribute("action", value);
-			return value;
 		}
 	};
 }
@@ -442,20 +460,6 @@ jQuery.each([ "selected", "checked", "readonly", "disabled" ], function( i, name
 	});
 });
 
-// elem.tabIndex doesn't always return the correct value when it hasn't been explicitly set
-// http://fluidproject.org/blog/2008/01/09/getting-setting-and-removing-tabindex-values-with-javascript/
-jQuery.attrHooks[ jQuery.attrFix.tabindex || "tabindex" ] = {
-	get: function( elem ) {
-		var attributeNode = elem.getAttributeNode("tabIndex");
-
-		return attributeNode && attributeNode.specified ?
-			parseInt( attributeNode.value, 10 ) :
-			rfocusable.test( elem.nodeName ) || rclickable.test( elem.nodeName ) && elem.href ?
-				0 :
-				undefined;
-	}
-};
-
 // Some attributes require a special call on IE
 if ( !jQuery.support.hrefNormalized ) {
 	jQuery.each([ "href", "src", "style" ], function( i, name ) {

test/index.html

@@ -203,7 +203,8 @@ Z</textarea>
 			<select name="D4" disabled="disabled">
 				<option selected="selected" value="NO">NO</option>
 			</select>
-			<input name="id">
+			<input name="id" />
+			<input id="target" name="target" />
 		</form>
 		<div id="moretests">
 			<form>

test/unit/attributes.js

@@ -3,15 +3,18 @@ module("attributes", { teardown: moduleTeardown });
 var bareObj = function(value) { return value; };
 var functionReturningObj = function(value) { return (function() { return value; }); };
 
-if ( !jQuery.support.getSetAttribute ) {
-	test("jQuery.attrFix integrity test", function() {
-		expect(1);
 
-		//  This must be maintained and equal jQuery.attrFix when appropriate
+test("jQuery.attrFix integrity test", function() {
-		//  Ensure that accidental or erroneous property
+	expect(1);
-		//  overwrites don't occur
+	
-		//  This is simply for better code coverage and future proofing.
+	//  This must be maintained and equal jQuery.attrFix when appropriate
-		var propsShouldBe = {
+	//  Ensure that accidental or erroneous property
+	//  overwrites don't occur
+	//  This is simply for better code coverage and future proofing.
+	var propsShouldBe;
+	if ( !jQuery.support.getSetAttribute ) {
+		propsShouldBe = {
+			tabindex: "tabIndex",
 			"for": "htmlFor",
 			"class": "className",
 			readonly: "readOnly",
@@ -19,14 +22,17 @@ if ( !jQuery.support.getSetAttribute ) {
 			cellspacing: "cellSpacing",
 			rowspan: "rowSpan",
 			colspan: "colSpan",
-			tabindex: "tabIndex",
 			usemap: "useMap",
 			frameborder: "frameBorder"
 		};
+	} else {
+		propsShouldBe = {
+			tabindex: "tabIndex"
+		};
+	}
 
-		same(propsShouldBe, jQuery.attrFix, "jQuery.attrFix passes integrity check");
+	same(propsShouldBe, jQuery.attrFix, "jQuery.attrFix passes integrity check");
-	});
+});
-}
 
 test("prop(String, Object)", function() {
 	expect(19);
@@ -69,7 +75,7 @@ test("prop(String, Object)", function() {
 });
 
 test("attr(String)", function() {
-	expect(22);
+	expect(24);
 
 	equals( jQuery('#text1').attr('type'), "text", 'Check for type attribute' );
 	equals( jQuery('#radio1').attr('type'), "radio", 'Check for type attribute' );
@@ -84,7 +90,9 @@ test("attr(String)", function() {
 	ok( jQuery('#form').attr('action').indexOf("formaction") >= 0, 'Check for action attribute' );
 	// [7472] & [3113] (form contains an input with name="action" or name="id")
 	equals( jQuery('#form').attr('action','newformaction').attr('action'), 'newformaction', 'Check that action attribute was changed' );
-	equals( jQuery('#testForm').removeAttr('id').attr('id'), undefined, 'Test that id does not equal the input with name=id after id is removed [#7472]' );
+	equals( jQuery('#testForm').attr('target'), undefined, 'Retrieving target does not equal the input with name=target' );
+	equals( jQuery('#testForm').attr('target', 'newTarget').attr('target'), 'newTarget', 'Set target successfully on a form' );
+	equals( jQuery('#testForm').removeAttr('id').attr('id'), undefined, 'Retrieving id does not equal the input with name=id after id is removed [#7472]' );
 	
 	equals( jQuery('#text1').attr('maxlength'), '30', 'Check for maxlength attribute' );
 	equals( jQuery('#text1').attr('maxLength'), '30', 'Check for maxLength attribute' );