Permalink
Browse files

Fix #5571. Setters should treat `undefined` as a no-op and be chainable.

  • Loading branch information...
1 parent d511613 commit 6c2a501de40a5f6b3ad382e2d309e5a10fce04d0 @gibson042 gibson042 committed with dmethvin Dec 6, 2011
Showing with 248 additions and 229 deletions.
  1. +2 −2 src/attributes.js
  2. +40 −16 src/core.js
  3. +2 −7 src/css.js
  4. +35 −28 src/data.js
  5. +32 −41 src/dimensions.js
  6. +39 −47 src/manipulation.js
  7. +42 −67 src/offset.js
  8. +13 −7 src/queue.js
  9. +7 −1 test/unit/attributes.js
  10. +1 −2 test/unit/data.js
  11. +14 −4 test/unit/dimensions.js
  12. +10 −0 test/unit/manipulation.js
  13. +7 −5 test/unit/offset.js
  14. +4 −2 test/unit/queue.js
View
@@ -12,7 +12,7 @@ var rclass = /[\n\t\r]/g,
jQuery.fn.extend({
attr: function( name, value ) {
- return jQuery.access( this, name, value, true, jQuery.attr );
+ return jQuery.access( this, jQuery.attr, name, value, arguments.length > 1 );
},
removeAttr: function( name ) {
@@ -22,7 +22,7 @@ jQuery.fn.extend({
},
prop: function( name, value ) {
- return jQuery.access( this, name, value, true, jQuery.prop );
+ return jQuery.access( this, jQuery.prop, name, value, arguments.length > 1 );
},
removeProp: function( name ) {
View
@@ -801,31 +801,55 @@ jQuery.extend({
// Mutifunctional method to get and set values to a collection
// The value/s can optionally be executed if it's a function
- access: function( elems, key, value, exec, fn, pass ) {
- var length = elems.length;
+ access: function( elems, fn, key, value, chainable, emptyGet, pass ) {
+ var exec,
+ bulk = key == null,
@mdomba

mdomba Dec 7, 2011

Sorry to jump here and excuse me if I'm wrong, but I'm not clear why is the variable "key" created here and set to null... will not that prevent the access to the same name parameter "key"

@rdworth

rdworth Dec 7, 2011

Contributor

this line sets bulk equal to the binary result of doing a double equals == comparison of key and null, meaning bulk will be true if key is null or undefined, false if any other value

@mdomba

mdomba Dec 7, 2011

Yes, you are right... I overlooked the '==' , thank you for the explanation

+ i = 0,
+ length = elems.length;
- // Setting many attributes
- if ( typeof key === "object" ) {
- for ( var k in key ) {
- jQuery.access( elems, k, key[k], exec, fn, value );
+ // Sets many values
+ if ( key && typeof key === "object" ) {
+ for ( i in key ) {
+ jQuery.access( elems, fn, i, key[i], 1, emptyGet, value );
}
- return elems;
- }
+ chainable = 1;
- // Setting one attribute
- if ( value !== undefined ) {
+ // Sets one value
+ } else if ( value !== undefined ) {
// Optionally, function values get executed if exec is true
- exec = !pass && exec && jQuery.isFunction(value);
+ exec = pass === undefined && jQuery.isFunction( value );
+
+ if ( bulk ) {
+ // Bulk operations only iterate when executing function values
+ if ( exec ) {
+ exec = fn;
+ fn = function( elem, key, value ) {
+ return exec.call( jQuery( elem ), value );
+ };
- for ( var i = 0; i < length; i++ ) {
- fn( elems[i], key, exec ? value.call( elems[i], i, fn( elems[i], key ) ) : value, pass );
+ // Otherwise they run against the entire set
+ } else {
+ fn.call( elems, value );
+ fn = null;
+ }
}
- return elems;
+ if ( fn ) {
+ for (; i < length; i++ ) {
+ fn( elems[i], key, exec ? value.call( elems[i], i, fn( elems[i], key ) ) : value, pass );
+ }
+ }
+
+ chainable = 1;
}
- // Getting an attribute
- return length ? fn( elems[0], key ) : undefined;
+ return chainable ?
+ elems :
+
+ // Gets
+ bulk ?
+ fn.call( elems ) :
+ length ? fn( elems[0], key ) : emptyGet;
},
now: function() {
View
@@ -17,16 +17,11 @@ var ralpha = /alpha\([^)]*\)/i,
currentStyle;
jQuery.fn.css = function( name, value ) {
- // Setting 'undefined' is a no-op
- if ( arguments.length === 2 && value === undefined ) {
- return this;
- }
-
- return jQuery.access( this, name, value, true, function( elem, name, value ) {
+ return jQuery.access( this, function( elem, name, value ) {
return value !== undefined ?
jQuery.style( elem, name, value ) :
jQuery.css( elem, name );
- });
+ }, name, value, arguments.length > 1 );
};
jQuery.extend({
View
@@ -246,62 +246,69 @@ jQuery.extend({
jQuery.fn.extend({
data: function( key, value ) {
- var parts, attr, name,
+ var parts, part, attr, name, l,
+ elem = this[0],
+ i = 0,
data = null;
- if ( typeof key === "undefined" ) {
+ // Gets all values
+ if ( key === undefined ) {
if ( this.length ) {
- data = jQuery.data( this[0] );
+ data = jQuery.data( elem );
- if ( this[0].nodeType === 1 && !jQuery._data( this[0], "parsedAttrs" ) ) {
- attr = this[0].attributes;
- for ( var i = 0, l = attr.length; i < l; i++ ) {
+ if ( elem.nodeType === 1 && !jQuery._data( elem, "parsedAttrs" ) ) {
+ attr = elem.attributes;
+ for ( l = attr.length; i < l; i++ ) {
name = attr[i].name;
if ( name.indexOf( "data-" ) === 0 ) {
name = jQuery.camelCase( name.substring(5) );
- dataAttr( this[0], name, data[ name ] );
+ dataAttr( elem, name, data[ name ] );
}
}
- jQuery._data( this[0], "parsedAttrs", true );
+ jQuery._data( elem, "parsedAttrs", true );
}
}
return data;
+ }
- } else if ( typeof key === "object" ) {
+ // Sets multiple values
+ if ( typeof key === "object" ) {
return this.each(function() {
jQuery.data( this, key );
});
}
- parts = key.split(".");
- parts[1] = parts[1] ? "." + parts[1] : "";
+ return jQuery.access( this, function( value ) {
+ parts = key.split( ".", 2 ),
+ parts[1] = parts[1] ? "." + parts[1] : "";
+ part = parts[1] + "!";
- if ( value === undefined ) {
- data = this.triggerHandler("getData" + parts[1] + "!", [parts[0]]);
+ if ( value === undefined ) {
+ data = this.triggerHandler( "getData" + part, [ parts[0] ] );
- // Try to fetch any internally stored data first
- if ( data === undefined && this.length ) {
- data = jQuery.data( this[0], key );
- data = dataAttr( this[0], key, data );
- }
+ // Try to fetch any internally stored data first
+ if ( data === undefined && elem ) {
+ data = jQuery.data( elem, key );
+ data = dataAttr( elem, key, data );
+ }
- return data === undefined && parts[1] ?
- this.data( parts[0] ) :
- data;
+ return data === undefined && parts[1] ?
+ this.data( parts[0] ) :
+ data;
+ }
- } else {
- return this.each(function() {
- var self = jQuery( this ),
- args = [ parts[0], value ];
+ parts[1] = value;
+ this.each(function() {
+ var self = jQuery( this );
- self.triggerHandler( "setData" + parts[1] + "!", args );
+ self.triggerHandler( "setData" + part, parts );
jQuery.data( this, key, value );
- self.triggerHandler( "changeData" + parts[1] + "!", args );
+ self.triggerHandler( "changeData" + part, parts );
});
- }
+ }, null, value, arguments.length > 1, null, false );
},
removeData: function( key ) {
View
@@ -1,9 +1,10 @@
(function( jQuery ) {
// Create width, height, innerHeight, innerWidth, outerHeight and outerWidth methods
-jQuery.each([ "Height", "Width" ], function( i, name ) {
-
- var type = name.toLowerCase();
+jQuery.each( { Height: "height", Width: "width" }, function( name, type ) {
+ var clientProp = "client" + name,
+ scrollProp = "scroll" + name,
+ offsetProp = "offset" + name;
// innerHeight and innerWidth
jQuery.fn[ "inner" + name ] = function() {
@@ -25,50 +26,40 @@ jQuery.each([ "Height", "Width" ], function( i, name ) {
null;
};
- jQuery.fn[ type ] = function( size ) {
- // Get window width or height
- var elem = this[0];
- if ( !elem ) {
- return size == null ? null : this;
- }
-
- if ( jQuery.isFunction( size ) ) {
- return this.each(function( i ) {
- var self = jQuery( this );
- self[ type ]( size.call( this, i, self[ type ]() ) );
- });
- }
+ jQuery.fn[ type ] = function( value ) {
+ return jQuery.access( this, function( elem, type, value ) {
+ var doc, docElemProp, orig, ret;
- if ( jQuery.isWindow( elem ) ) {
- // Everyone else use document.documentElement or document.body depending on Quirks vs Standards mode
- // 3rd condition allows Nokia support, as it supports the docElem prop but not CSS1Compat
- var docElemProp = elem.document.documentElement[ "client" + name ],
- body = elem.document.body;
- return elem.document.compatMode === "CSS1Compat" && docElemProp ||
- body && body[ "client" + name ] || docElemProp;
+ if ( jQuery.isWindow( elem ) ) {
+ // 3rd condition allows Nokia support, as it supports the docElem prop but not CSS1Compat
+ doc = elem.document;
+ docElemProp = doc.documentElement[ clientProp ];
+ return doc.compatMode === "CSS1Compat" && docElemProp ||
+ doc.body && doc.body[ clientProp ] || docElemProp;
+ }
- // Get document width or height
- } else if ( elem.nodeType === 9 ) {
- // Either scroll[Width/Height] or offset[Width/Height], whichever is greater
- return Math.max(
- elem.documentElement["client" + name],
- elem.body["scroll" + name], elem.documentElement["scroll" + name],
- elem.body["offset" + name], elem.documentElement["offset" + name]
- );
+ // Get document width or height
+ if ( elem.nodeType === 9 ) {
+ // Either scroll[Width/Height] or offset[Width/Height], whichever is greater
+ doc = elem.documentElement;
+ return Math.max(
+ doc[ clientProp ],
+ elem.body[ scrollProp ], doc[ scrollProp ],
+ elem.body[ offsetProp ], doc[ offsetProp ]
+ );
+ }
- // Get or set width or height on the element
- } else if ( size === undefined ) {
- var orig = jQuery.css( elem, type ),
+ // Get width or height on the element
+ if ( value === undefined ) {
+ orig = jQuery.css( elem, type );
ret = parseFloat( orig );
+ return jQuery.isNumeric( ret ) ? ret : orig;
+ }
- return jQuery.isNumeric( ret ) ? ret : orig;
-
- // Set the width or height on the element (default to pixels if value is unitless)
- } else {
- return this.css( type, typeof size === "string" ? size : size + "px" );
- }
+ // Set the width or height on the element
+ jQuery( elem ).css( type, value );
+ }, type, value, arguments.length, null );
};
-
});
})( jQuery );
Oops, something went wrong.

0 comments on commit 6c2a501

Please sign in to comment.