Skip to content

Commit

Permalink
Landing pull request 500. 1.7 - "public data" stored as a key on "int…
Browse files Browse the repository at this point in the history 
…ernal data" - Fixes #8921.

More Details:
 - #500
 - http://bugs.jquery.com/ticket/8921
  • Loading branch information
@gnarf @timmywil
gnarf authored and timmywil committed Sep 19, 2011
1 parent e4c48a3 commit d5f144a
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 78 deletions.
61 changes: 23 additions & 38 deletions src/data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ jQuery.extend({

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

return !!elem && !isEmptyDataObject( elem );
},

Expand All @@ -51,7 +50,7 @@ jQuery.extend({

// 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 ] && !cache[ id ][ internalKey ]))) && getByName && data === undefined ) {
if ( (!id || !cache[id] || (!pvt && !cache[id].data)) && getByName && data === undefined ) {
return;
}

Expand All @@ -68,9 +67,8 @@ jQuery.extend({
if ( !cache[ id ] ) {
cache[ id ] = {};

// TODO: This is a hack for 1.5 ONLY. Avoids exposing jQuery
// metadata on plain JS objects when the object is serialized using
// JSON.stringify
// Avoids exposing jQuery metadata on plain JS objects when the object
// is serialized using JSON.stringify
if ( !isNode ) {
cache[ id ].toJSON = jQuery.noop;
}
Expand All @@ -80,23 +78,23 @@ jQuery.extend({
// shallow copied over onto the existing cache
if ( typeof name === "object" || typeof name === "function" ) {
if ( pvt ) {
cache[ id ][ internalKey ] = jQuery.extend(cache[ id ][ internalKey ], name);
cache[ id ] = jQuery.extend( cache[ id ], name );
} else {
cache[ id ] = jQuery.extend(cache[ id ], name);
cache[ id ].data = jQuery.extend( cache[ id ].data, name );
}
}

thisCache = cache[ id ];

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

thisCache = thisCache[ internalKey ];
thisCache = thisCache.data;
}

if ( data !== undefined ) {
Expand Down Expand Up @@ -156,7 +154,7 @@ jQuery.extend({

if ( name ) {

thisCache = pvt ? cache[ id ][ internalKey ] : cache[ id ];
thisCache = pvt ? cache[ id ] : cache[ id ].data;

if ( thisCache ) {

Expand All @@ -169,15 +167,15 @@ jQuery.extend({

// If there is no data left in the cache, we want to continue
// and let the cache object itself get destroyed
if ( !isEmptyDataObject(thisCache) ) {
if ( !( pvt ? isEmptyDataObject : jQuery.isEmptyObject )( thisCache ) ) {
return;
}
}
}

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

// Don't destroy the parent cache unless the internal data object
// had been the only thing left in it
Expand All @@ -186,8 +184,6 @@ jQuery.extend({
}
}

var internalCache = cache[ id ][ internalKey ];

// 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
Expand All @@ -198,23 +194,9 @@ jQuery.extend({
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 ] = {};
// TODO: This is a hack for 1.5 ONLY. Avoids exposing jQuery
// metadata on plain JS objects when the object is serialized using
// JSON.stringify
if ( !isNode ) {
cache[ id ].toJSON = jQuery.noop;
}

cache[ id ][ internalKey ] = internalCache;

// Otherwise, we need to eliminate the expando on the node to avoid
// We destroyed the cache and need to eliminate the expando on the node to avoid
// false lookups in the cache for entries that no longer exist
} else if ( isNode ) {
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
Expand Down Expand Up @@ -342,11 +324,14 @@ function dataAttr( elem, key, data ) {
return data;
}

// TODO: This is a hack for 1.5 ONLY to allow objects with a single toJSON
// property to be considered empty objects; this property always exists in
// order to make sure JSON.stringify does not expose internal metadata
// checks a cache object for emptiness
function isEmptyDataObject( obj ) {
for ( var name in obj ) {

// if the public data object is empty, the private is still empty
if ( name === "data" && jQuery.isEmptyObject( obj[name] ) ) {
continue;
}
if ( name !== "toJSON" ) {
return false;
}
Expand Down
22 changes: 9 additions & 13 deletions src/event.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,10 @@ jQuery.event = {
}

delete elemData.events;
delete elemData.handle;

if ( jQuery.isEmptyObject( elemData ) ) {
jQuery.removeData( elem, undefined, true );
}
// removeData also checks for emptiness and clears the expando if empty
// so use it instead of delete for this last property we touch here
jQuery.removeData( elem, "handle", true );
}
}
},
Expand Down Expand Up @@ -326,17 +325,14 @@ jQuery.event = {

// Handle a global trigger
if ( !elem ) {

// TODO: Stop taunting the data cache; remove global events and always attach to document
jQuery.each( jQuery.cache, function() {
// internalKey variable is just used to make it easier to find
// and potentially change this stuff later; currently it just
// points to jQuery.expando
var internalKey = jQuery.expando,
internalCache = this[ internalKey ];
if ( internalCache && internalCache.events && internalCache.events[ type ] ) {
jQuery.event.trigger( event, data, internalCache.handle.elem );
var cache = jQuery.cache;
for ( i in cache ) {
if ( cache[ i ].events && cache[ i ].events[ type ] ) {
jQuery.event.trigger( event, data, cache[ i ].handle.elem );
}
});
}
return;
}

Expand Down
41 changes: 21 additions & 20 deletions src/manipulation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,27 +356,26 @@ function cloneCopyEvent( src, dest ) {
return;
}

var internalKey = jQuery.expando,
oldData = jQuery.data( src ),
curData = jQuery.data( dest, oldData );

// Switch to use the internal data object, if it exists, for the next
// stage of data copying
if ( (oldData = oldData[ internalKey ]) ) {
var events = oldData.events;
curData = curData[ internalKey ] = jQuery.extend({}, oldData);

if ( events ) {
delete curData.handle;
curData.events = {};

for ( var type in events ) {
for ( var i = 0, l = events[ type ].length; i < l; i++ ) {
jQuery.event.add( dest, type + ( events[ type ][ i ].namespace ? "." : "" ) + events[ type ][ i ].namespace, events[ type ][ i ], events[ type ][ i ].data );
}
var type, i, l,
oldData = jQuery._data( src ),
curData = jQuery._data( dest, oldData ),
events = oldData.events;

if ( events ) {
delete curData.handle;
curData.events = {};

for ( type in events ) {
for ( i = 0, l = events[ type ].length; i < l; i++ ) {
jQuery.event.add( dest, type + ( events[ type ][ i ].namespace ? "." : "" ) + events[ type ][ i ].namespace, events[ type ][ i ], events[ type ][ i ].data );
}
}
}

// make the cloned public data object a copy from the original
if ( curData.data ) {
curData.data = jQuery.extend( {}, curData.data );
}
}

function cloneFixAttributes( src, dest ) {
Expand Down Expand Up @@ -706,7 +705,9 @@ jQuery.extend({
},

cleanData: function( elems ) {
var data, id, cache = jQuery.cache, internalKey = jQuery.expando, special = jQuery.event.special,
var data, id,
cache = jQuery.cache,
special = jQuery.event.special,
deleteExpando = jQuery.support.deleteExpando;

for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) {
Expand All @@ -717,7 +718,7 @@ jQuery.extend({
id = elem[ jQuery.expando ];

if ( id ) {
data = cache[ id ] && cache[ id ][ internalKey ];
data = cache[ id ];

if ( data && data.events ) {
for ( var type in data.events ) {
Expand Down
15 changes: 8 additions & 7 deletions test/unit/data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test("expando", function(){
});

function dataTests (elem) {
// expect(32)
// expect(31)

function getCacheLength() {
var cacheLength = 0;
Expand Down Expand Up @@ -52,8 +52,8 @@ function dataTests (elem) {
equals( jQuery._data(elem, "foo"), "foo2", "Setting internal data works" );
equals( jQuery.data(elem, "foo"), "foo1", "Setting internal data does not override user data" );

var internalDataObj = jQuery.data(elem, jQuery.expando);
strictEqual( jQuery._data(elem), internalDataObj, "Internal data object is accessible via jQuery.expando property" );
var internalDataObj = jQuery._data( elem );
ok( internalDataObj, "Internal data object exists" );
notStrictEqual( dataObj, internalDataObj, "Internal data object is not the same as user data object" );

strictEqual( elem.boom, undefined, "Data is never stored directly on the object" );
Expand All @@ -62,7 +62,7 @@ function dataTests (elem) {
strictEqual( jQuery.data(elem, "foo"), undefined, "jQuery.removeData removes single properties" );

jQuery.removeData(elem);
strictEqual( jQuery.data(elem, jQuery.expando), internalDataObj, "jQuery.removeData does not remove internal data if it exists" );
strictEqual( jQuery._data(elem), internalDataObj, "jQuery.removeData does not remove internal data if it exists" );

jQuery.removeData(elem, undefined, true);

Expand All @@ -75,8 +75,9 @@ function dataTests (elem) {
jQuery.data(elem, "foo", "foo1");
equals( jQuery._data(elem, "foo"), "foo2", "Setting user data does not override internal data" );

jQuery.removeData(elem, undefined, true);
equals( jQuery.data(elem, "foo"), "foo1", "jQuery.removeData for internal data does not remove user data" );
// delete the last private data key so we can test removing public data
// will destroy the cache
jQuery.removeData( elem, "foo", true );

if (elem.nodeType) {
var oldCacheLength = getCacheLength();
Expand Down Expand Up @@ -138,7 +139,7 @@ function dataTests (elem) {
}

test("jQuery.data", function() {
expect(128);
expect(124);

var div = document.createElement("div");

Expand Down

0 comments on commit d5f144a

Please sign in to comment.