Skip to content

Commit

Permalink
Make the new attr/prop changes pass the test suite (in Webkit). There…
Browse files Browse the repository at this point in the history
… are still errors in IE.

+ Added hooks for selected, checked, readonly, disabled to removeAttr if set to falsey

+ Removed all attrs from attrFix, these aren't needed for setAttribute

+ If prop is used for class, do we want a propFix for class?

  - We could just assume the user should know to use className with prop.  I've done the latter for now.

+ Created tests for $.fn.prop and $.fn.removeProp

  - Actually all I did was change broken attr tests to prop where it made sense.
  • Loading branch information
timmywil committed Apr 3, 2011
1 parent ab4e300 commit de79e8c
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 111 deletions.
48 changes: 28 additions & 20 deletions src/attributes.js
Expand Up @@ -36,10 +36,10 @@ jQuery.fn.extend({
}, },


addClass: function( value ) { addClass: function( value ) {
if ( jQuery.isFunction(value) ) { if ( jQuery.isFunction( value ) ) {
return this.each(function(i) { return this.each(function(i) {
var self = jQuery(this); var self = jQuery(this);
self.addClass( value.call(this, i, self.attr("class")) ); self.addClass( value.call(this, i, self.attr("class") || "") );
}); });
} }


Expand Down Expand Up @@ -278,19 +278,10 @@ jQuery.extend({
// TODO: Check to see if any of these are needed anymore? // TODO: Check to see if any of these are needed anymore?
// If not, it may be good to standardize on all-lowercase names instead // If not, it may be good to standardize on all-lowercase names instead
attrFix: { attrFix: {
"for": "htmlFor",
"class": "className",
readonly: "readOnly",
maxlength: "maxLength",
cellspacing: "cellSpacing",
rowspan: "rowSpan",
colspan: "colSpan",
tabindex: "tabIndex",
usemap: "useMap",
frameborder: "frameBorder"
}, },


attr: function( elem, name, value, pass ) { attr: function( elem, name, value, pass ) {

// don't get/set attributes on text, comment and attribute nodes // don't get/set attributes on text, comment and attribute nodes
if ( !elem || elem.nodeType === 3 || elem.nodeType === 8 || elem.nodeType === 2 ) { if ( !elem || elem.nodeType === 3 || elem.nodeType === 8 || elem.nodeType === 2 ) {
return undefined; return undefined;
Expand All @@ -314,6 +305,10 @@ jQuery.extend({
if ( hooks && "set" in hooks && notxml && (ret = hooks.set( elem, value )) !== undefined ) { if ( hooks && "set" in hooks && notxml && (ret = hooks.set( elem, value )) !== undefined ) {
return ret; return ret;


} else if ( value === null ) {
elem.removeAttribute( name );
return undefined;

} else { } else {
// convert the value to a string (all browsers do this but IE) see #1070 // convert the value to a string (all browsers do this but IE) see #1070
value = "" + value; value = "" + value;
Expand All @@ -329,24 +324,22 @@ jQuery.extend({
return ret; return ret;


} else { } else {

if ( !jQuery.hasAttr( elem, name ) ) { if ( !jQuery.hasAttr( elem, name ) ) {
return undefined; return undefined;
} }


var attr = elem.getAttribute( name ); var attr = elem.getAttribute( name );


// Non-existent attributes return null, we normalize to undefined // Non-existent attributes return null, we normalize to undefined
return attr === null ? return attr === null ? undefined : attr;
undefined :
attr;
} }
} }
}, },


hasAttr: function( elem, name ) { hasAttr: function( elem, name ) {
// Blackberry 4.7 returns "" from getAttribute #6938 // Blackberry 4.7 returns "" from getAttribute #6938
return elem.attributes[ name ] || (elem.hasAttribute && elem.hasAttribute( name )); return elem && elem.attributes[ name ] || (elem.hasAttribute && elem.hasAttribute( name ));
}, },


attrHooks: { attrHooks: {
Expand All @@ -361,7 +354,7 @@ jQuery.extend({


// elem.tabIndex doesn't always return the correct value when it hasn't been explicitly set // 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/ // http://fluidproject.org/blog/2008/01/09/getting-setting-and-removing-tabindex-values-with-javascript/
tabIndex: { tabindex: {
get: function( elem ) { get: function( elem ) {
var attributeNode = elem.getAttributeNode( "tabIndex" ); var attributeNode = elem.getAttributeNode( "tabIndex" );


Expand All @@ -375,10 +368,11 @@ jQuery.extend({
}, },


// TODO: Check to see if we really need any here. // TODO: Check to see if we really need any here.
propFix: {}, propFix: {
},


prop: function( elem, name, value ) { prop: function( elem, name, value ) {
var ret, hooks; var ret, hooks, notxml = elem.nodeType !== 1 || !jQuery.isXMLDoc( elem );


// Try to normalize/fix the name // Try to normalize/fix the name
name = notxml && jQuery.propFix[ name ] || name; name = notxml && jQuery.propFix[ name ] || name;
Expand Down Expand Up @@ -406,6 +400,20 @@ jQuery.extend({
propHooks: {} propHooks: {}
}); });


// Remove certain attrs if set to false
jQuery.each([ "selected", "checked", "readonly", "disabled" ], function( i, name ) {
jQuery.attrHooks[ name ] = jQuery.extend( jQuery.attrHooks[ name ], {
set: function( elem, value ) {
if ( !value ) { // '', undefined, false, null will remove attr
elem.removeAttribute( name );
return false;
}
elem.setAttribute( name, value );
return value;
}
});
});

// Some attributes require a special call on IE // Some attributes require a special call on IE
if ( !jQuery.support.hrefNormalized ) { if ( !jQuery.support.hrefNormalized ) {
jQuery.each([ "href", "src", "style" ], function( i, name ) { jQuery.each([ "href", "src", "style" ], function( i, name ) {
Expand Down
2 changes: 1 addition & 1 deletion src/manipulation.js
Expand Up @@ -377,7 +377,7 @@ function cloneCopyEvent( src, dest ) {
} }
} }


function cloneFixAttributes(src, dest) { function cloneFixAttributes( src, dest ) {
// We do not need to do anything for non-Elements // We do not need to do anything for non-Elements
if ( dest.nodeType !== 1 ) { if ( dest.nodeType !== 1 ) {
return; return;
Expand Down

0 comments on commit de79e8c

Please sign in to comment.