Skip to content
Permalink
Browse files

Merge in data_nocollide branch. Fixes #6968, improves unit testing fr…

…amework checks for leaky stuff.
  • Loading branch information
csnover committed Jan 17, 2011
2 parents f01ef93 + 57cc182 commit e78d3a7e2d47e9796f87c18b76f8178b0bee42c5
@@ -133,11 +133,11 @@ jQuery.fn.extend({
} else if ( type === "undefined" || type === "boolean" ) {
if ( this.className ) {
// store className if set
jQuery.data( this, "__className__", this.className );
jQuery._data( this, "__className__", this.className );
}

// toggle whole className
this.className = this.className || value === false ? "" : jQuery.data( this, "__className__" ) || "";
this.className = this.className || value === false ? "" : jQuery._data( this, "__className__" ) || "";
}
});
},
@@ -1,7 +1,6 @@
(function( jQuery ) {

var windowData = {},
rbrace = /^(?:\{.*\}|\[.*\])$/;
var rbrace = /^(?:\{.*\}|\[.*\])$/;

jQuery.extend({
cache: {},
@@ -23,110 +22,170 @@ jQuery.extend({
},

hasData: function( elem ) {
if ( elem.nodeType ) {
elem = jQuery.cache[ elem[jQuery.expando] ];
}
elem = elem.nodeType ? jQuery.cache[ elem[jQuery.expando] ] : elem[ jQuery.expando ];

return !!elem && !jQuery.isEmptyObject(elem);
},

data: function( elem, name, data ) {
data: function( elem, name, data, pvt /* Internal Use Only */ ) {
if ( !jQuery.acceptData( elem ) ) {
return;
}

elem = elem == window ?
windowData :
elem;
var internalKey = jQuery.expando, getByName = typeof name === "string", thisCache,

// We have to handle DOM nodes and JS objects differently because IE6-7
// can't GC object references properly across the DOM-JS boundary
isNode = elem.nodeType,

// Only DOM nodes need the global jQuery cache; JS object data is
// attached directly to the object so GC can occur automatically
cache = isNode ? jQuery.cache : elem,

var isNode = elem.nodeType,
id = isNode ? elem[ jQuery.expando ] : null,
cache = jQuery.cache, thisCache;
// Only defining an ID for JS objects if its cache already exists allows
// the code to shortcut on the same path as a DOM node with no cache
id = isNode ? elem[ jQuery.expando ] : elem[ jQuery.expando ] && jQuery.expando;

if ( isNode && !id && typeof name === "string" && data === undefined ) {
// Avoid doing any more work than we need to when trying to get data on an
// object that has no data at all
if ( (!id || (pvt && id && !cache[ id ][ internalKey ])) && getByName && data === undefined ) {
return;
}

// Get the data from the object directly
if ( !isNode ) {
cache = elem;
if ( !id ) {
// Only DOM nodes need a new unique ID for each element since their data
// ends up in the global cache
if ( isNode ) {
elem[ jQuery.expando ] = id = ++jQuery.uuid;
} else {
id = jQuery.expando;
}
}

// Compute a unique ID for the element
} else if ( !id ) {
elem[ jQuery.expando ] = id = ++jQuery.uuid;
if ( !cache[ id ] ) {
cache[ id ] = {};
}

// Avoid generating a new cache unless none exists and we
// want to manipulate it.
// An object can be passed to jQuery.data instead of a key/value pair; this gets
// shallow copied over onto the existing cache
if ( typeof name === "object" ) {
if ( isNode ) {
if ( pvt ) {
cache[ id ][ internalKey ] = jQuery.extend(cache[ id ][ internalKey ], name);
} else {
cache[ id ] = jQuery.extend(cache[ id ], name);
}
}

} else {
jQuery.extend( cache, name );
thisCache = cache[ id ];

// Internal jQuery data is stored in a separate object inside the object's data
// cache in order to avoid key collisions between internal data and user-defined
// data
if ( pvt ) {
if ( !thisCache[ internalKey ] ) {
thisCache[ internalKey ] = {};
}

} else if ( isNode && !cache[ id ] ) {
cache[ id ] = {};
thisCache = thisCache[ internalKey ];
}

thisCache = isNode ? cache[ id ] : cache;

// Prevent overriding the named cache with undefined values
if ( data !== undefined ) {
thisCache[ name ] = data;
}

return typeof name === "string" ? thisCache[ name ] : thisCache;
// TODO: This is a hack for 1.5 ONLY. It will be removed in 1.6. Users should
// not attempt to inspect the internal events object using jQuery.data, as this
// internal data object is undocumented and subject to change.
if ( name === "events" && !thisCache[name] ) {
return thisCache[ internalKey ] && thisCache[ internalKey ].events;
}

return getByName ? thisCache[ name ] : thisCache;
},

removeData: function( elem, name ) {
removeData: function( elem, name, pvt /* Internal Use Only */ ) {
if ( !jQuery.acceptData( elem ) ) {
return;
}

elem = elem == window ?
windowData :
elem;
var internalKey = jQuery.expando, isNode = elem.nodeType,

// See jQuery.data for more information
cache = isNode ? jQuery.cache : elem,

var isNode = elem.nodeType,
id = isNode ? elem[ jQuery.expando ] : elem,
cache = jQuery.cache,
thisCache = isNode ? cache[ id ] : id;
// See jQuery.data for more information
id = isNode ? elem[ jQuery.expando ] : jQuery.expando;

// If there is already no cache entry for this object, there is no
// purpose in continuing
if ( !cache[ id ] ) {
return;
}

// If we want to remove a specific section of the element's data
if ( name ) {
var thisCache = pvt ? cache[ id ][ internalKey ] : cache[ id ];

if ( thisCache ) {
// Remove the section of cache data
delete thisCache[ name ];

// If we've removed all the data, remove the element's cache
if ( isNode && jQuery.isEmptyObject(thisCache) ) {
jQuery.removeData( elem );
// If there is no data left in the cache, we want to continue
// and let the cache object itself get destroyed
if ( !jQuery.isEmptyObject(thisCache) ) {
return;
}
}
}

// See jQuery.data for more information
if ( pvt ) {
delete cache[ id ][ internalKey ];

// Don't destroy the parent cache unless the internal data object
// had been the only thing left in it
if ( !jQuery.isEmptyObject(cache[ id ]) ) {
return;
}
}

var internalCache = cache[ id ][ internalKey ];

// Otherwise, we want to remove all of the element's data
// Browsers that fail expando deletion also refuse to delete expandos on
// the window, but it will allow it on all other JS objects; other browsers
// don't care
if ( jQuery.support.deleteExpando || cache != window ) {
delete cache[ id ];
} else {
if ( isNode && jQuery.support.deleteExpando ) {
delete elem[ jQuery.expando ];
cache[ id ] = null;
}

// We destroyed the entire user cache at once because it's faster than
// iterating through each key, but we need to continue to persist internal
// data if it existed
if ( internalCache ) {
cache[ id ] = {};
cache[ id ][ internalKey ] = internalCache;

// Otherwise, we need to eliminate the expando on the node to avoid
// false lookups in the cache for entries that no longer exist
} else if ( isNode ) {
// IE does not allow us to delete expando properties from nodes,
// nor does it have a removeAttribute function on Document nodes;
// we must handle all of these cases
if ( jQuery.support.deleteExpando ) {
delete elem[ jQuery.expando ];
} else if ( elem.removeAttribute ) {
elem.removeAttribute( jQuery.expando );

// Completely remove the data cache
} else if ( isNode ) {
delete cache[ id ];

// Remove all fields from the object
} else {
for ( var n in elem ) {
delete elem[ n ];
}
elem[ jQuery.expando ] = null;
}
}
},

// For internal use only.
_data: function( elem, name, data ) {
return jQuery.data( elem, name, data, true );
},

// A method for determining if a DOM node can handle the data expando
acceptData: function( elem ) {
if ( elem.nodeName ) {
@@ -27,15 +27,15 @@ jQuery.fn.extend({

// Reset the inline display of this element to learn if it is
// being hidden by cascaded rules or not
if ( !jQuery.data(elem, "olddisplay") && display === "none" ) {
if ( !jQuery._data(elem, "olddisplay") && display === "none" ) {
display = elem.style.display = "";
}

// Set elements which have been overridden with display: none
// in a stylesheet to whatever the default browser style is
// for such an element
if ( display === "" && jQuery.css( elem, "display" ) === "none" ) {
jQuery.data(elem, "olddisplay", defaultDisplay(elem.nodeName));
jQuery._data(elem, "olddisplay", defaultDisplay(elem.nodeName));
}
}

@@ -46,7 +46,7 @@ jQuery.fn.extend({
display = elem.style.display;

if ( display === "" || display === "none" ) {
elem.style.display = jQuery.data(elem, "olddisplay") || "";
elem.style.display = jQuery._data(elem, "olddisplay") || "";
}
}

@@ -62,8 +62,8 @@ jQuery.fn.extend({
for ( var i = 0, j = this.length; i < j; i++ ) {
var display = jQuery.css( this[i], "display" );

if ( display !== "none" && !jQuery.data( this[i], "olddisplay" ) ) {
jQuery.data( this[i], "olddisplay", display );
if ( display !== "none" && !jQuery._data( this[i], "olddisplay" ) ) {
jQuery._data( this[i], "olddisplay", display );
}
}

3 comments on commit e78d3a7

@algor

This comment has been minimized.

Copy link

algor replied Jan 18, 2011

Maybe I am wrong but it looks like the changes effectively mean that for plain JS objects following is no longer true:
var obj = { name: "test" };
jQuery(obj).data("name") === obj.name; // is not true any more

But what about official jquery-datalink plugin?
And overall, regardless of the any plugin functionality, do you think it is justified to store any non-private data to obj[jQuery.expando] for plain JS objects instead of directly as properties of the object?
I completely agree that it is safer this way to store any internal information, like event bindings, within separate suboject. But why it is needed to break logical and justified behavior when the rest of the data keys are stored directly as object properties?

@csnover

This comment has been minimized.

Copy link
Member Author

csnover replied Jan 18, 2011

@algor: You’re right! The behaviour here changes. jQuery.data is really for storing metadata on an object/element and so shouldn’t really meddle directly with properties. That was a change introduced between 1.4.2 and 1.4.3 in order to avoid memory leaks, and it is being restored to its original behaviour (w/o memory leaks) in 1.5. The datalink plugin is being rewritten from the ground-up and will not be using this functionality any more. :)

@algor

This comment has been minimized.

Copy link

algor replied Jan 18, 2011

Colin, thank you very much for the clarifications!

Please sign in to comment.
You can’t perform that action at this time.