From d5f144a7bba98212d6fe9e257f722b62baf651f0 Mon Sep 17 00:00:00 2001 From: Corey Frang Date: Mon, 19 Sep 2011 16:13:14 -0400 Subject: [PATCH] Landing pull request 500. 1.7 - "public data" stored as a key on "internal data" - Fixes #8921. More Details: - https://github.com/jquery/jquery/pull/500 - http://bugs.jquery.com/ticket/8921 --- src/data.js | 61 +++++++++++++++++---------------------------- src/event.js | 22 +++++++--------- src/manipulation.js | 41 +++++++++++++++--------------- test/unit/data.js | 15 +++++------ 4 files changed, 61 insertions(+), 78 deletions(-) diff --git a/src/data.js b/src/data.js index d1d2d0032..9de0da1c5 100644 --- a/src/data.js +++ b/src/data.js @@ -24,7 +24,6 @@ jQuery.extend({ hasData: function( elem ) { elem = elem.nodeType ? jQuery.cache[ elem[jQuery.expando] ] : elem[ jQuery.expando ]; - return !!elem && !isEmptyDataObject( elem ); }, @@ -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; } @@ -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; } @@ -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 ) { @@ -156,7 +154,7 @@ jQuery.extend({ if ( name ) { - thisCache = pvt ? cache[ id ][ internalKey ] : cache[ id ]; + thisCache = pvt ? cache[ id ] : cache[ id ].data; if ( thisCache ) { @@ -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 @@ -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 @@ -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 @@ -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; } diff --git a/src/event.js b/src/event.js index 363448b60..2617382ad 100644 --- a/src/event.js +++ b/src/event.js @@ -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 ); } } }, @@ -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; } diff --git a/src/manipulation.js b/src/manipulation.js index 2f41feb9a..35c9544ff 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -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 ) { @@ -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++ ) { @@ -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 ) { diff --git a/test/unit/data.js b/test/unit/data.js index b1bf232a4..ebeb1b4ee 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -7,7 +7,7 @@ test("expando", function(){ }); function dataTests (elem) { - // expect(32) + // expect(31) function getCacheLength() { var cacheLength = 0; @@ -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" ); @@ -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); @@ -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(); @@ -138,7 +139,7 @@ function dataTests (elem) { } test("jQuery.data", function() { - expect(128); + expect(124); var div = document.createElement("div"); -- 2.39.5