Landing pull request 500. 1.7 - "public data" stored as a key on "internal data"...

More Details:
 - https://github.com/jquery/jquery/pull/500
 - http://bugs.jquery.com/ticket/8921

diff --git a/src/data.js b/src/data.js
index d1d2d0032c1010b8afc20e9a6f19d67bb9fda403..9de0da1c543180152c8efdcd6ba3c4bde4e06d7f 100644 (file)
--- 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 363448b600ba34d7d8759e3a4b4a1eba7384a474..2617382ad830cc588a71778323257b51e6a4a7c2 100644 (file)
--- 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 2f41feb9a8bfc8c6098ea41c2102f97754124ca6..35c9544ffa003b2df2222b60e88e84a0b9ca7c80 100644 (file)
--- 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 b1bf232a4040df4186044aac9e04de98ff2761d5..ebeb1b4ee940c259b7f1e9a00ca198afd238a545 100644 (file)
--- 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");