From 9e05a0a37fcc6fc4985a95370f91e1f792e2273b Mon Sep 17 00:00:00 2001 From: timmywil Date: Fri, 25 Mar 2011 01:46:29 -0400 Subject: [PATCH] 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 | 86 +++++++++++++++++++++-------------------- test/index.html | 3 +- test/unit/attributes.js | 36 ++++++++++------- 3 files changed, 69 insertions(+), 56 deletions(-) diff --git a/src/attributes.js b/src/attributes.js index 564eaeeb..3c6dee8b 100644 --- a/src/attributes.js +++ b/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") ) { - return ret; + if ( hooks && "get" in hooks && notxml ) { + 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) - // setting className to null makes the class "null" - if ( name === "className" ) { - elem[ name ] = ""; - } else { - elem.setAttribute( name, null ); - } + // use DOM level 1 if getSetAttribute not supported (IE6-7) + elem.setAttribute( name, "" ); // Set to default empty string + elem.removeAttributeNode( elem.getAttributeNode( name ) ); } }, @@ -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 - jQuery.attrHooks.action = { - get: function( elem ) { - return elem.nodeName === "FORM" ? - elem.getAttributeNode("action").nodeValue : - elem.getAttribute("action"); - }, - set: function( elem, value ) { - elem.nodeName === "FORM" ? - elem.getAttributeNode("action").nodeValue = value : - elem.setAttribute("action", value); - return value; + + // Name attribute will not get removed in browsers that do not support getSetAttribute + // Return undefined on empty string or null + jQuery.attrHooks.name = { + get: function( elem, value ) { + return elem.getAttributeNode("name").nodeValue || undefined; } }; } @@ -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 ) { diff --git a/test/index.html b/test/index.html index 4a8aef52..584f1f86 100644 --- a/test/index.html +++ b/test/index.html @@ -203,7 +203,8 @@ Z - + +
diff --git a/test/unit/attributes.js b/test/unit/attributes.js index 5b614446..adc1d591 100644 --- a/test/unit/attributes.js +++ b/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 - // Ensure that accidental or erroneous property - // overwrites don't occur - // This is simply for better code coverage and future proofing. - var propsShouldBe = { +test("jQuery.attrFix integrity test", function() { + expect(1); + + // This must be maintained and equal jQuery.attrFix when appropriate + // 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' );