From 8e59a99e0ade75dec434f246f52e8b3f7393f359 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sun, 9 Jan 2011 15:52:33 -0600 Subject: [PATCH 1/5] Change the way jQuery.data works so that there is no longer a chance of collision between user data and internal data. Fixes #6968. --- src/attributes.js | 4 +- src/data.js | 184 +++++++++++++++++++----------- src/effects.js | 10 +- src/event.js | 46 ++++---- src/manipulation.js | 31 +++-- src/queue.js | 4 +- test/unit/attributes.js | 10 +- test/unit/data.js | 242 +++++++++++++++++++++++++++------------- test/unit/effects.js | 4 +- test/unit/event.js | 20 ++-- 10 files changed, 352 insertions(+), 203 deletions(-) diff --git a/src/attributes.js b/src/attributes.js index fec132340..d37400a68 100644 --- a/src/attributes.js +++ b/src/attributes.js @@ -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__" ) || ""; } }); }, diff --git a/src/data.js b/src/data.js index 4d1d1bd55..a1abc9ed6 100644 --- a/src/data.js +++ b/src/data.js @@ -1,7 +1,6 @@ (function( jQuery ) { -var windowData = {}, - rbrace = /^(?:\{.*\}|\[.*\])$/; +var rbrace = /^(?:\{.*\}|\[.*\])$/; jQuery.extend({ cache: {}, @@ -23,108 +22,161 @@ 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, - var isNode = elem.nodeType, - id = isNode ? elem[ jQuery.expando ] : null, - cache = jQuery.cache, 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, - if ( isNode && !id && typeof name === "string" && data === undefined ) { + // 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, + + // 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; + + // 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; - - // Compute a unique ID for the element - } else if ( !id ) { - elem[ jQuery.expando ] = id = ++jQuery.uuid; + 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; + } } - // Avoid generating a new cache unless none exists and we - // want to manipulate it. - if ( typeof name === "object" ) { - if ( isNode ) { - cache[ id ] = jQuery.extend(cache[ id ], name); - - } else { - jQuery.extend( cache, name ); - } - - } else if ( isNode && !cache[ id ] ) { + if ( !cache[ id ] ) { cache[ id ] = {}; } - thisCache = isNode ? cache[ id ] : cache; + // 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 ( pvt ) { + cache[ id ][ internalKey ] = jQuery.extend(cache[ id ][ internalKey ], name); + } else { + cache[ id ] = jQuery.extend(cache[ id ], 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 ] = {}; + } + + thisCache = thisCache[ internalKey ]; + } - // Prevent overriding the named cache with undefined values if ( data !== undefined ) { thisCache[ name ] = data; } - return typeof name === "string" ? thisCache[ name ] : thisCache; + 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, - var isNode = elem.nodeType, - id = isNode ? elem[ jQuery.expando ] : elem, - cache = jQuery.cache, - thisCache = isNode ? cache[ id ] : id; + // See jQuery.data for more information + cache = isNode ? jQuery.cache : elem, + + // 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 ); - } - } - - // Otherwise, we want to remove all of the element's data - } else { - if ( isNode && 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 ]; + // 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 ]; + + // 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 { + 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 ); + } else { + 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 diff --git a/src/effects.js b/src/effects.js index bd57ffc3d..b0675395f 100644 --- a/src/effects.js +++ b/src/effects.js @@ -27,7 +27,7 @@ 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 = ""; } @@ -35,7 +35,7 @@ jQuery.fn.extend({ // 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 ); } } diff --git a/src/event.js b/src/event.js index 675e5fff3..7fa5488ee 100644 --- a/src/event.js +++ b/src/event.js @@ -8,7 +8,8 @@ var rnamespaces = /\.(.*)$/, fcleanup = function( nm ) { return nm.replace(rescape, "\\$&"); }, - focusCounts = { focusin: 0, focusout: 0 }; + focusCounts = { focusin: 0, focusout: 0 }, + eventKey = "events"; /* * A number of helper functions used for managing events. @@ -50,7 +51,7 @@ jQuery.event = { } // Init the element's event structure - var elemData = jQuery.data( elem ); + var elemData = jQuery._data( elem ); // If no elemData is found then we must be trying to bind to one of the // banned noData elements @@ -58,10 +59,7 @@ jQuery.event = { return; } - // Use a key less likely to result in collisions for plain JS objects. - // Fixes bug #7150. - var eventKey = elem.nodeType ? "events" : "__events__", - events = elemData[ eventKey ], + var events = elemData[ eventKey ], eventHandle = elemData.handle; if ( typeof events === "function" ) { @@ -177,8 +175,7 @@ jQuery.event = { } var ret, type, fn, j, i = 0, all, namespaces, namespace, special, eventType, handleObj, origType, - eventKey = elem.nodeType ? "events" : "__events__", - elemData = jQuery.data( elem ), + elemData = jQuery.hasData( elem ) && jQuery._data( elem ), events = elemData && elemData[ eventKey ]; if ( !elemData || !events ) { @@ -290,10 +287,10 @@ jQuery.event = { delete elemData.handle; if ( typeof elemData === "function" ) { - jQuery.removeData( elem, eventKey ); + jQuery.removeData( elem, eventKey, true ); } else if ( jQuery.isEmptyObject( elemData ) ) { - jQuery.removeData( elem ); + jQuery.removeData( elem, undefined, true ); } } }, @@ -325,9 +322,16 @@ jQuery.event = { // Only trigger if we've ever bound an event for it if ( jQuery.event.global[ type ] ) { + // XXX This code smells terrible. event.js should not be directly + // inspecting the data cache jQuery.each( jQuery.cache, function() { - if ( this.events && this.events[type] ) { - jQuery.event.trigger( event, data, this.handle.elem ); + // 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 ); } }); } @@ -353,8 +357,8 @@ jQuery.event = { // Trigger the event, it is assumed that "handle" is a function var handle = elem.nodeType ? - jQuery.data( elem, "handle" ) : - (jQuery.data( elem, "__events__" ) || {}).handle; + jQuery._data( elem, "handle" ) : + (jQuery._data( elem, eventKey ) || {}).handle; if ( handle ) { handle.apply( elem, data ); @@ -432,7 +436,7 @@ jQuery.event = { event.namespace = event.namespace || namespace_sort.join("."); - events = jQuery.data(this, this.nodeType ? "events" : "__events__"); + events = jQuery._data(this, eventKey); if ( typeof events === "function" ) { events = events.events; @@ -787,12 +791,12 @@ if ( !jQuery.support.changeBubbles ) { return; } - data = jQuery.data( elem, "_change_data" ); + data = jQuery._data( elem, "_change_data" ); val = getVal(elem); // the current data will be also retrieved by beforeactivate if ( e.type !== "focusout" || elem.type !== "radio" ) { - jQuery.data( elem, "_change_data", val ); + jQuery._data( elem, "_change_data", val ); } if ( data === undefined || val === data ) { @@ -837,7 +841,7 @@ if ( !jQuery.support.changeBubbles ) { // information beforeactivate: function( e ) { var elem = e.target; - jQuery.data( elem, "_change_data", getVal(elem) ); + jQuery._data( elem, "_change_data", getVal(elem) ); } }, @@ -986,8 +990,8 @@ jQuery.fn.extend({ return this.click( jQuery.proxy( fn, function( event ) { // Figure out which function to execute - var lastToggle = ( jQuery.data( this, "lastToggle" + fn.guid ) || 0 ) % i; - jQuery.data( this, "lastToggle" + fn.guid, lastToggle + 1 ); + var lastToggle = ( jQuery._data( this, "lastToggle" + fn.guid ) || 0 ) % i; + jQuery._data( this, "lastToggle" + fn.guid, lastToggle + 1 ); // Make sure that clicks stop event.preventDefault(); @@ -1075,7 +1079,7 @@ function liveHandler( event ) { var stop, maxLevel, related, match, handleObj, elem, j, i, l, data, close, namespace, ret, elems = [], selectors = [], - events = jQuery.data( this, this.nodeType ? "events" : "__events__" ); + events = jQuery._data( this, eventKey ); if ( typeof events === "function" ) { events = events.events; diff --git a/src/manipulation.js b/src/manipulation.js index 96caa02d0..657aef7d1 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -381,17 +381,24 @@ function cloneCopyEvent(orig, ret) { throw "Cloned data mismatch"; } - var oldData = jQuery.data( orig[nodeIndex] ), - curData = jQuery.data( this, oldData ), - events = oldData && oldData.events; + var internalKey = jQuery.expando, + oldData = jQuery.data( orig[nodeIndex] ), + curData = jQuery.data( this, oldData ); - if ( events ) { - delete curData.handle; - curData.events = {}; + // 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); - for ( var type in events ) { - for ( var i = 0, l = events[ type ].length; i < l; i++ ) { - jQuery.event.add( this, type, events[ type ][ i ], events[ type ][ i ].data ); + 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( this, type, events[ type ][ i ], events[ type ][ i ].data ); + } } } } @@ -594,8 +601,7 @@ jQuery.extend({ }, cleanData: function( elems ) { - var data, id, cache = jQuery.cache, - special = jQuery.event.special, + var data, id, cache = jQuery.cache, internalKey = jQuery.expando, special = jQuery.event.special, deleteExpando = jQuery.support.deleteExpando; for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) { @@ -606,13 +612,14 @@ jQuery.extend({ id = elem[ jQuery.expando ]; if ( id ) { - data = cache[ id ]; + data = cache[ id ] && cache[ id ][ internalKey ]; if ( data && data.events ) { for ( var type in data.events ) { if ( special[ type ] ) { jQuery.event.remove( elem, type ); + // This is a shortcut to avoid jQuery.event.remove's overhead } else { jQuery.removeEvent( elem, type, data.handle ); } diff --git a/src/queue.js b/src/queue.js index 735b0e189..5fb04df80 100644 --- a/src/queue.js +++ b/src/queue.js @@ -7,7 +7,7 @@ jQuery.extend({ } type = (type || "fx") + "queue"; - var q = jQuery.data( elem, type ); + var q = jQuery._data( elem, type ); // Speed up dequeue by getting out quickly if this is just a lookup if ( !data ) { @@ -15,7 +15,7 @@ jQuery.extend({ } if ( !q || jQuery.isArray(data) ) { - q = jQuery.data( elem, type, jQuery.makeArray(data) ); + q = jQuery._data( elem, type, jQuery.makeArray(data) ); } else { q.push( data ); diff --git a/test/unit/attributes.js b/test/unit/attributes.js index a1ab58179..04f168466 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -703,12 +703,12 @@ var testToggleClass = function(valueObj) { // toggleClass storage e.toggleClass(true); - ok( e.get(0).className === "", "Assert class is empty (data was empty)" ); + ok( e[0].className === "", "Assert class is empty (data was empty)" ); e.addClass("testD testE"); ok( e.is(".testD.testE"), "Assert class present" ); e.toggleClass(); ok( !e.is(".testD.testE"), "Assert class not present" ); - ok( e.data('__className__') === 'testD testE', "Assert data was stored" ); + ok( jQuery._data(e[0], '__className__') === 'testD testE', "Assert data was stored" ); e.toggleClass(); ok( e.is(".testD.testE"), "Assert class present (restored from data)" ); e.toggleClass(false); @@ -720,11 +720,9 @@ var testToggleClass = function(valueObj) { e.toggleClass(); ok( e.is(".testD.testE"), "Assert class present (restored from data)" ); - - // Cleanup e.removeClass("testD"); - e.removeData('__className__'); + jQuery.removeData(e[0], '__className__', true); }; test("toggleClass(String|boolean|undefined[, boolean])", function() { @@ -785,7 +783,7 @@ test("toggleClass(Fucntion[, boolean]) with incoming value", function() { // Cleanup e.removeClass("test"); - e.removeData('__className__'); + jQuery.removeData(e[0], '__className__', true); }); test("addClass, removeClass, hasClass", function() { diff --git a/test/unit/data.js b/test/unit/data.js index 310cd6bc4..28e19a3e2 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -1,22 +1,159 @@ -module("data"); +module("data", { teardown: moduleTeardown }); test("expando", function(){ - expect(6); + expect(1); equals("expando" in jQuery, true, "jQuery is exposing the expando"); +}); - var obj = {}; - equals( jQuery.data(obj), obj, "jQuery.data(obj) returns the object"); - equals( jQuery.expando in obj, false, "jQuery.data(obj) did not add an expando to the object" ); +function dataTests (elem) { + // expect(32) - obj = {}; - jQuery.data(obj, 'test'); - equals( jQuery.expando in obj, false, "jQuery.data(obj,key) did not add an expando to the object" ); + function getCacheLength() { + var cacheLength = 0; + for (var i in jQuery.cache) { + ++cacheLength; + } - obj = {}; - jQuery.data(obj, "foo", "bar"); - equals( jQuery.expando in obj, false, "jQuery.data(obj,key,value) did not add an expando to the object" ); - equals( obj.foo, "bar", "jQuery.data(obj,key,value) sets fields directly on the object." ); + return cacheLength; + } + + equals( jQuery.data(elem, "foo"), undefined, "No data exists initially" ); + strictEqual( jQuery.hasData(elem), false, "jQuery.hasData agrees no data exists initially" ); + + var dataObj = jQuery.data(elem); + equals( typeof dataObj, "object", "Calling data with no args gives us a data object reference" ); + strictEqual( jQuery.data(elem), dataObj, "Calling jQuery.data returns the same data object when called multiple times" ); + + strictEqual( jQuery.hasData(elem), false, "jQuery.hasData agrees no data exists even when an empty data obj exists" ); + + dataObj.foo = "bar"; + equals( jQuery.data(elem, "foo"), "bar", "Data is readable by jQuery.data when set directly on a returned data object" ); + + strictEqual( jQuery.hasData(elem), true, "jQuery.hasData agrees data exists when data exists" ); + + jQuery.data(elem, "foo", "baz"); + equals( jQuery.data(elem, "foo"), "baz", "Data can be changed by jQuery.data" ); + equals( dataObj.foo, "baz", "Changes made through jQuery.data propagate to referenced data object" ); + + jQuery.data(elem, "foo", undefined); + equals( jQuery.data(elem, "foo"), "baz", "Data is not unset by passing undefined to jQuery.data" ); + + jQuery.data(elem, "foo", null); + strictEqual( jQuery.data(elem, "foo"), null, "Setting null using jQuery.data works OK" ); + + jQuery.data(elem, "foo", "foo1"); + + jQuery.data(elem, { "bar" : "baz", "boom" : "bloz" }); + strictEqual( jQuery.data(elem, "foo"), "foo1", "Passing an object extends the data object instead of replacing it" ); + equals( jQuery.data(elem, "boom"), "bloz", "Extending the data object works" ); + + jQuery._data(elem, "foo", "foo2"); + 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" ); + 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" ); + + jQuery.removeData(elem, "foo"); + 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" ); + + jQuery.removeData(elem, undefined, true); + + strictEqual( jQuery.data(elem, jQuery.expando), undefined, "jQuery.removeData on internal data works" ); + strictEqual( jQuery.hasData(elem), false, "jQuery.hasData agrees all data has been removed from object" ); + + jQuery._data(elem, "foo", "foo2"); + strictEqual( jQuery.hasData(elem), true, "jQuery.hasData shows data exists even if it is only internal data" ); + + 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" ); + + if (elem.nodeType) { + var oldCacheLength = getCacheLength(); + jQuery.removeData(elem, "foo"); + + equals( getCacheLength(), oldCacheLength - 1, "Removing the last item in the data object destroys it" ); + } + else { + jQuery.removeData(elem, "foo"); + var expected, actual; + + if (jQuery.support.deleteExpando) { + expected = false; + actual = jQuery.expando in elem; + } + else { + expected = null; + actual = elem[ jQuery.expando ]; + } + + equals( actual, expected, "Removing the last item in the data object destroys it" ); + } + + jQuery.data(elem, "foo", "foo1"); + jQuery._data(elem, "foo", "foo2"); + + equals( jQuery.data(elem, "foo"), "foo1", "(sanity check) Ensure data is set in user data object" ); + equals( jQuery._data(elem, "foo"), "foo2", "(sanity check) Ensure data is set in internal data object" ); + + jQuery.removeData(elem, "foo", true); + + strictEqual( jQuery.data(elem, jQuery.expando), undefined, "Removing the last item in internal data destroys the internal data object" ); + + jQuery._data(elem, "foo", "foo2"); + equals( jQuery._data(elem, "foo"), "foo2", "(sanity check) Ensure data is set in internal data object" ); + + jQuery.removeData(elem, "foo"); + equals( jQuery._data(elem, "foo"), "foo2", "(sanity check) jQuery.removeData for user data does not remove internal data" ); + + if (elem.nodeType) { + oldCacheLength = getCacheLength(); + jQuery.removeData(elem, "foo", true); + equals( getCacheLength(), oldCacheLength - 1, "Removing the last item in the internal data object also destroys the user data object when it is empty" ); + } + else { + jQuery.removeData(elem, "foo", true); + + if (jQuery.support.deleteExpando) { + expected = false; + actual = jQuery.expando in elem; + } + else { + expected = null; + actual = elem[ jQuery.expando ]; + } + + equals( actual, expected, "Removing the last item in the internal data object also destroys the user data object when it is empty" ); + } +} + +test("jQuery.data", function() { + expect(128); + + var div = document.createElement("div"); + + dataTests(div); + dataTests({}); + + // remove bound handlers from window object to stop potential false positives caused by fix for #5280 in + // transports/xhr.js + jQuery(window).unbind("unload"); + + dataTests(window); + dataTests(document); + + // clean up unattached element + jQuery(div).remove(); }); test("jQuery.acceptData", function() { @@ -37,68 +174,11 @@ test("jQuery.acceptData", function() { ok( !jQuery.acceptData( applet ), "applet" ); }); -test("jQuery.data", function() { - expect(15); - var div = document.createElement("div"); - - ok( jQuery.data(div, "test") === undefined, "Check for no data exists" ); - - jQuery.data(div, "test", "success"); - equals( jQuery.data(div, "test"), "success", "Check for added data" ); - - ok( jQuery.data(div, "notexist") === undefined, "Check for no data exists" ); - - var data = jQuery.data(div); - same( data, { "test": "success" }, "Return complete data set" ); - - jQuery.data(div, "test", "overwritten"); - equals( jQuery.data(div, "test"), "overwritten", "Check for overwritten data" ); - - jQuery.data(div, "test", undefined); - equals( jQuery.data(div, "test"), "overwritten", "Check that data wasn't removed"); - - jQuery.data(div, "test", null); - ok( jQuery.data(div, "test") === null, "Check for null data"); - - jQuery.data(div, "test3", "orig"); - jQuery.data(div, { "test": "in", "test2": "in2" }); - equals( jQuery.data(div, "test"), "in", "Verify setting an object in data" ); - equals( jQuery.data(div, "test2"), "in2", "Verify setting an object in data" ); - equals( jQuery.data(div, "test3"), "orig", "Verify original not overwritten" ); - - var obj = {}; - jQuery.data( obj, "prop", true ); - - ok( obj.prop, "Data is being stored on the object" ); - equals( jQuery.data( obj, "prop" ), true, "Make sure the right value is retrieved" ); - - jQuery.data( window, "BAD", true ); - ok( !window[ jQuery.expando ], "Make sure there is no expando on the window object." ); - ok( !window.BAD, "And make sure that the property wasn't set directly on the window." ); - ok( jQuery.data( window, "BAD" ), "Make sure that the value was set." ); -}); - -test("jQuery.hasData", function() { - expect(6); - - function testData(obj) { - equals( jQuery.hasData(obj), false, "No data exists" ); - jQuery.data( obj, "foo", "bar" ); - equals( jQuery.hasData(obj), true, "Data exists" ); - jQuery.removeData( obj, "foo" ); - equals( jQuery.hasData(obj), false, "Data was removed" ); - } - - testData(document.createElement('div')); - testData({}); -}); - test(".data()", function() { expect(5); var div = jQuery("#foo"); strictEqual( div.data("foo"), undefined, "Make sure that missing result is undefined" ); - div.data("test", "success"); same( div.data(), {test: "success"}, "data() get the entire data object" ); strictEqual( div.data("foo"), undefined, "Make sure that missing result is still undefined" ); @@ -107,7 +187,7 @@ test(".data()", function() { equals( nodiv.data(), null, "data() on empty set returns null" ); var obj = { foo: "bar" }; - equals( jQuery(obj).data(), obj, "Retrieve data object from a wrapped JS object (#7524)" ); + deepEqual( jQuery(obj).data(), {}, "Retrieve data object from a wrapped JS object (#7524)" ); }) test(".data(String) and .data(String, Object)", function() { @@ -194,11 +274,14 @@ test(".data(String) and .data(String, Object)", function() { equals( $elem.data('null',null).data('null'), null, "null's are preserved"); equals( $elem.data('emptyString','').data('emptyString'), '', "Empty strings are preserved"); equals( $elem.data('false',false).data('false'), false, "false's are preserved"); - equals( $elem.data('exists'), true, "Existing data is returned" ); + equals( $elem.data('exists'), undefined, "Existing data is not returned" ); // Clean up $elem.removeData(); - ok( jQuery.isEmptyObject( $elem[0] ), "removeData clears the object" ); + deepEqual( $elem[0], {exists:true}, "removeData does not clear the object" ); + + // manually clean up detached elements + parent.remove(); }); test("data-* attributes", function() { @@ -323,13 +406,17 @@ test(".data(Object)", function() { var obj = {test:"unset"}, jqobj = jQuery(obj); + jqobj.data("test", "unset"); jqobj.data({ "test": "in", "test2": "in2" }); - equals( obj.test, "in", "Verify setting an object on an object extends the object" ); - equals( obj.test2, "in2", "Verify setting an object on an object extends the object" ); + equals( jQuery.data(obj).test, "in", "Verify setting an object on an object extends the data object" ); + equals( obj.test2, undefined, "Verify setting an object on an object does not extend the object" ); + + // manually clean up detached elements + div.remove(); }); test("jQuery.removeData", function() { - expect(7); + expect(6); var div = jQuery("#foo")[0]; jQuery.data(div, "test", "testing"); jQuery.removeData(div, "test"); @@ -342,10 +429,9 @@ test("jQuery.removeData", function() { var obj = {}; jQuery.data(obj, "test", "testing"); - equals( obj.test, "testing", "verify data on plain object"); + equals( jQuery(obj).data("test"), "testing", "verify data on plain object"); jQuery.removeData(obj, "test"); equals( jQuery.data(obj, "test"), undefined, "Check removal of data on plain object" ); - equals( obj.test, undefined, "Check removal of data directly from plain object" ); jQuery.data( window, "BAD", true ); jQuery.removeData( window, "BAD" ); @@ -371,4 +457,4 @@ test(".removeData()", function() { div.removeData("test.foo"); equals( div.data("test.foo"), undefined, "Make sure data is intact" ); -}); +}); \ No newline at end of file diff --git a/test/unit/effects.js b/test/unit/effects.js index b7b60abbe..cf9d13109 100644 --- a/test/unit/effects.js +++ b/test/unit/effects.js @@ -895,7 +895,7 @@ test("hide hidden elements (bug #7141)", function() { var div = jQuery("
").appendTo("#main"); equals( div.css("display"), "none", "Element is hidden by default" ); div.hide(); - ok( !div.data("olddisplay"), "olddisplay is undefined after hiding an already-hidden element" ); + ok( !jQuery._data(div, "olddisplay"), "olddisplay is undefined after hiding an already-hidden element" ); div.show(); equals( div.css("display"), "block", "Show a double-hidden element" ); @@ -910,7 +910,7 @@ test("hide hidden elements, with animation (bug #7141)", function() { var div = jQuery("
").appendTo("#main"); equals( div.css("display"), "none", "Element is hidden by default" ); div.hide(1, function () { - ok( !div.data("olddisplay"), "olddisplay is undefined after hiding an already-hidden element" ); + ok( !jQuery._data(div, "olddisplay"), "olddisplay is undefined after hiding an already-hidden element" ); div.show(1, function () { equals( div.css("display"), "block", "Show a double-hidden element" ); start(); diff --git a/test/unit/event.js b/test/unit/event.js index b4672a8b8..cae1a83fe 100644 --- a/test/unit/event.js +++ b/test/unit/event.js @@ -28,7 +28,7 @@ test("bind(), with data", function() { }; jQuery("#firstp").bind("click", {foo: "bar"}, handler).click().unbind("click", handler); - ok( !jQuery.data(jQuery("#firstp")[0], "events"), "Event handler unbound when using data." ); + ok( !jQuery._data(jQuery("#firstp")[0], "events"), "Event handler unbound when using data." ); }); test("click(), with data", function() { @@ -39,7 +39,7 @@ test("click(), with data", function() { }; jQuery("#firstp").click({foo: "bar"}, handler).click().unbind("click", handler); - ok( !jQuery.data(jQuery("#firstp")[0], "events"), "Event handler unbound when using data." ); + ok( !jQuery._data(jQuery("#firstp")[0], "events"), "Event handler unbound when using data." ); }); test("bind(), with data, trigger with data", function() { @@ -505,7 +505,7 @@ test("bind(), with different this object", function() { .bind("click", jQuery.proxy(handler1, thisObject)).click().unbind("click", handler1) .bind("click", data, jQuery.proxy(handler2, thisObject)).click().unbind("click", handler2); - ok( !jQuery.data(jQuery("#firstp")[0], "events"), "Event handler unbound when using different this object and data." ); + ok( !jQuery._data(jQuery("#firstp")[0], "events"), "Event handler unbound when using different this object and data." ); }); test("bind(name, false), unbind(name, false)", function() { @@ -547,7 +547,7 @@ test("bind()/trigger()/unbind() on plain object", function() { } }); - var events = jQuery(obj).data("__events__"); + var events = jQuery._data(obj, "events"); ok( events, "Object has events bound." ); equals( obj.events, undefined, "Events object on plain objects is not events" ); equals( typeof events, "function", "'events' expando is a function on plain objects." ); @@ -567,7 +567,9 @@ test("bind()/trigger()/unbind() on plain object", function() { // Make sure it doesn't complain when no events are found jQuery(obj).unbind("test"); - equals( obj.__events__, undefined, "Make sure events object is removed" ); + equals( obj && obj[ jQuery.expando ] && + obj[ jQuery.expando ][ jQuery.expando ] && + obj[ jQuery.expando ][ jQuery.expando ].events, undefined, "Make sure events object is removed" ); }); test("unbind(type)", function() { @@ -947,7 +949,7 @@ test("toggle(Function, Function, ...)", function() { equals( turn, 2, "Trying toggle with 3 functions, attempt 5 yields 2"); $div.unbind('click',fns[0]); - var data = jQuery.data( $div[0], 'events' ); + var data = jQuery._data( $div[0], 'events' ); ok( !data, "Unbinding one function from toggle unbinds them all"); // Test Multi-Toggles @@ -1065,7 +1067,7 @@ test(".live()/.die()", function() { equals( clicked, 2, "live with a context" ); // Make sure the event is actually stored on the context - ok( jQuery.data(container, "events").live, "live with a context" ); + ok( jQuery._data(container, "events").live, "live with a context" ); // Test unbinding with a different context jQuery("#foo", container).die("click"); @@ -1578,7 +1580,7 @@ test(".delegate()/.undelegate()", function() { equals( clicked, 2, "delegate with a context" ); // Make sure the event is actually stored on the context - ok( jQuery.data(container, "events").live, "delegate with a context" ); + ok( jQuery._data(container, "events").live, "delegate with a context" ); // Test unbinding with a different context jQuery("#main").undelegate("#foo", "click"); @@ -1907,7 +1909,7 @@ test("window resize", function() { ok( true, "Resize event fired." ); }).resize().unbind("resize"); - ok( !jQuery(window).data("__events__"), "Make sure all the events are gone." ); + ok( !jQuery._data(window, "__events__"), "Make sure all the events are gone." ); }); test("focusin bubbles", function() { From 885d06c8ef906fa11d130d7d567c871d20ef9ba9 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sun, 9 Jan 2011 15:56:40 -0600 Subject: [PATCH 2/5] Fix domManip leaks the first element when appending elements to multiple other elements. --- src/manipulation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manipulation.js b/src/manipulation.js index 657aef7d1..206476c2d 100644 --- a/src/manipulation.js +++ b/src/manipulation.js @@ -346,7 +346,7 @@ jQuery.fn.extend({ table ? root(this[i], first) : this[i], - i > 0 || results.cacheable || this.length > 1 ? + i > 0 || results.cacheable || (this.length > 1 && i > 0) ? jQuery(fragment).clone(true)[0] : fragment ); From 80af46e8ffe8292e0af0537db6c7e89019e5edba Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sun, 9 Jan 2011 15:58:23 -0600 Subject: [PATCH 3/5] Fix jQuery.queue leaks empty queues. --- src/queue.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/queue.js b/src/queue.js index 5fb04df80..9e3e2fb52 100644 --- a/src/queue.js +++ b/src/queue.js @@ -46,6 +46,10 @@ jQuery.extend({ jQuery.dequeue(elem, type); }); } + + if ( !queue.length ) { + jQuery.removeData( elem, type + "queue", true ); + } } }); From e2941d5a98e91c5f61b200b2763e5fa0eb339365 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sun, 9 Jan 2011 15:58:47 -0600 Subject: [PATCH 4/5] Update unit tests with a leak detection mechanism for the various jQuery globals and fix all leaks in the tests. --- test/data/testinit.js | 49 ++++++++++++++++++++++++++++++ test/index.html | 2 +- test/unit/ajax.js | 2 +- test/unit/attributes.js | 2 +- test/unit/core.js | 5 +++- test/unit/css.js | 2 +- test/unit/data.js | 4 +++ test/unit/dimensions.js | 18 ++++++++++- test/unit/effects.js | 31 ++++++++++++------- test/unit/event.js | 41 ++++++++++++++++++++++--- test/unit/manipulation.js | 63 +++++++++++++++++++++++++++++++-------- test/unit/offset.js | 2 +- test/unit/queue.js | 2 +- test/unit/selector.js | 2 +- test/unit/traversing.js | 2 +- 15 files changed, 189 insertions(+), 38 deletions(-) diff --git a/test/data/testinit.js b/test/data/testinit.js index a66f71d25..c478390d5 100644 --- a/test/data/testinit.js +++ b/test/data/testinit.js @@ -45,3 +45,52 @@ function t(a,b,c) { function url(value) { return value + (/\?/.test(value) ? "&" : "?") + new Date().getTime() + "" + parseInt(Math.random()*100000); } + +(function () { + // Store the old counts so that we only assert on tests that have actually leaked, + // instead of asserting every time a test has leaked sometime in the past + var oldCacheLength = 0, + oldFragmentsLength = 0, + oldTimersLength = 0, + oldActive = 0; + + /** + * Ensures that tests have cleaned up properly after themselves. Should be passed as the + * teardown function on all modules' lifecycle object. + */ + this.moduleTeardown = function () { + var i, fragmentsLength = 0, cacheLength = 0; + + // Allow QUnit.reset to clean up any attached elements before checking for leaks + QUnit.reset(); + + for ( i in jQuery.cache ) { + ++cacheLength; + } + + jQuery.fragments = {}; + + for ( i in jQuery.fragments ) { + ++fragmentsLength; + } + + // Because QUnit doesn't have a mechanism for retrieving the number of expected assertions for a test, + // if we unconditionally assert any of these, the test will fail with too many assertions :| + if ( cacheLength !== oldCacheLength ) { + equals( cacheLength, oldCacheLength, "No unit tests leak memory in jQuery.cache" ); + oldCacheLength = cacheLength; + } + if ( fragmentsLength !== oldFragmentsLength ) { + equals( fragmentsLength, oldFragmentsLength, "No unit tests leak memory in jQuery.fragments" ); + oldFragmentsLength = fragmentsLength; + } + if ( jQuery.timers.length !== oldTimersLength ) { + equals( jQuery.timers.length, oldTimersLength, "No timers are still running" ); + oldTimersLength = jQuery.timers.length; + } + if ( jQuery.active !== oldActive ) { + equals( jQuery.active, 0, "No AJAX requests are still active" ); + oldActive = jQuery.active; + } + } +}()); \ No newline at end of file diff --git a/test/index.html b/test/index.html index bbeda63a6..fc5f667d2 100644 --- a/test/index.html +++ b/test/index.html @@ -264,7 +264,7 @@ Z
slideToggleOut
slideToggleOut
fadeToggleIn
fadeToggleIn
-
fadeToggleOut
fadeToggleOut
+
fadeToggleOut
fadeToggleOut
fadeTo
fadeTo
diff --git a/test/unit/ajax.js b/test/unit/ajax.js index 773088fc9..d29eddbfa 100644 --- a/test/unit/ajax.js +++ b/test/unit/ajax.js @@ -1,4 +1,4 @@ -module("ajax"); +module("ajax", { teardown: moduleTeardown }); // Safari 3 randomly crashes when running these tests, // but only in the full suite - you can run just the Ajax diff --git a/test/unit/attributes.js b/test/unit/attributes.js index 04f168466..c58111de1 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -1,4 +1,4 @@ -module("attributes"); +module("attributes", { teardown: moduleTeardown }); var bareObj = function(value) { return value; }; var functionReturningObj = function(value) { return (function() { return value; }); }; diff --git a/test/unit/core.js b/test/unit/core.js index 8fd060578..6231b1d53 100644 --- a/test/unit/core.js +++ b/test/unit/core.js @@ -1,4 +1,4 @@ -module("core"); +module("core", { teardown: moduleTeardown }); test("Basic requirements", function() { expect(7); @@ -84,6 +84,9 @@ test("jQuery()", function() { exec = true; elem.click(); + + // manually clean up detached elements + elem.remove(); }); test("selector state", function() { diff --git a/test/unit/css.js b/test/unit/css.js index fbbf937ca..063707196 100644 --- a/test/unit/css.js +++ b/test/unit/css.js @@ -1,4 +1,4 @@ -module("css"); +module("css", { teardown: moduleTeardown }); test("css(String|Hash)", function() { expect(41); diff --git a/test/unit/data.js b/test/unit/data.js index 28e19a3e2..889fc2da3 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -301,6 +301,8 @@ test("data-* attributes", function() { div.data("attr", "internal").attr("data-attr", "external"); equals( div.data("attr"), "internal", "Check for .data('attr') precedence (internal > external data-* attribute)" ); + div.remove(); + child.appendTo('#main'); equals( child.data("myobj"), "old data", "Value accessed from data-* attribute"); @@ -312,6 +314,8 @@ test("data-* attributes", function() { var obj = child.data(), obj2 = dummy.data(), check = [ "myobj", "ignored", "other" ], num = 0, num2 = 0; + dummy.remove(); + for ( var i = 0, l = check.length; i < l; i++ ) { ok( obj[ check[i] ], "Make sure data- property exists when calling data-." ); ok( obj2[ check[i] ], "Make sure data- property exists when calling data-." ); diff --git a/test/unit/dimensions.js b/test/unit/dimensions.js index b38e73bba..fa59a9f77 100644 --- a/test/unit/dimensions.js +++ b/test/unit/dimensions.js @@ -1,4 +1,4 @@ -module("dimensions"); +module("dimensions", { teardown: moduleTeardown }); function pass( val ) { return val; @@ -33,6 +33,8 @@ function testWidth( val ) { var blah = jQuery("blah"); equals( blah.width( val(10) ), blah, "Make sure that setting a width on an empty set returns the set." ); equals( blah.width(), null, "Make sure 'null' is returned on an empty set"); + + jQuery.removeData($div[0], 'olddisplay', true); } test("width()", function() { @@ -80,6 +82,8 @@ function testHeight( val ) { var blah = jQuery("blah"); equals( blah.height( val(10) ), blah, "Make sure that setting a height on an empty set returns the set." ); equals( blah.height(), null, "Make sure 'null' is returned on an empty set"); + + jQuery.removeData($div[0], 'olddisplay', true); } test("height()", function() { @@ -126,6 +130,9 @@ test("innerWidth()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.innerWidth(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("innerHeight()", function() { @@ -152,6 +159,9 @@ test("innerHeight()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.innerHeight(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("outerWidth()", function() { @@ -179,6 +189,9 @@ test("outerWidth()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.outerWidth(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("outerHeight()", function() { @@ -205,4 +218,7 @@ test("outerHeight()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.outerHeight(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); diff --git a/test/unit/effects.js b/test/unit/effects.js index cf9d13109..b1dd28840 100644 --- a/test/unit/effects.js +++ b/test/unit/effects.js @@ -1,4 +1,4 @@ -module("effects"); +module("effects", { teardown: moduleTeardown }); test("sanity check", function() { expect(1); @@ -14,7 +14,7 @@ test("show()", function() { equals( hiddendiv.css("display"), "block", "Make sure a pre-hidden div is visible." ); - var div = jQuery("
").hide().appendTo("body").show(); + var div = jQuery("
").hide().appendTo("#main").show(); equal( div.css("display"), "block", "Make sure pre-hidden divs show" ); @@ -403,13 +403,16 @@ test("animate duration 0", function() { $elem.hide(0, function(){ ok(true, "Hide callback with no duration"); }); + + // manually clean up detached elements + $elem.remove(); }); test("animate hyphenated properties", function(){ expect(1); stop(); - jQuery("#nothiddendiv") + jQuery("#foo") .css("font-size", 10) .animate({"font-size": 20}, 200, function(){ equals( this.style.fontSize, "20px", "The font-size property was animated." ); @@ -433,7 +436,7 @@ test("stop()", function() { expect(3); stop(); - var $foo = jQuery("#nothiddendiv"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -446,6 +449,8 @@ test("stop()", function() { nw = $foo.width(); notEqual( nw, w, "Stop didn't reset the animation " + nw + "px " + w + "px"); setTimeout(function(){ + $foo.removeData(); + $foo.removeData(undefined, true); equals( nw, $foo.width(), "The animation didn't continue" ); start(); }, 100); @@ -456,7 +461,7 @@ test("stop() - several in queue", function() { expect(3); stop(); - var $foo = jQuery("#nothiddendivchild"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -481,7 +486,7 @@ test("stop(clearQueue)", function() { expect(4); stop(); - var $foo = jQuery("#nothiddendiv"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -508,7 +513,7 @@ test("stop(clearQueue, gotoEnd)", function() { expect(1); stop(); - var $foo = jQuery("#nothiddendivchild"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -536,7 +541,7 @@ test("stop(clearQueue, gotoEnd)", function() { test("toggle()", function() { expect(6); - var x = jQuery("#nothiddendiv"); + var x = jQuery("#foo"); ok( x.is(":visible"), "is visible" ); x.toggle(); ok( x.is(":hidden"), "is hidden" ); @@ -737,6 +742,9 @@ jQuery.each( { } } + // manually remove generated element + jQuery(this).remove(); + start(); }); }); @@ -763,6 +771,10 @@ jQuery.checkState = function(){ var cur = self.style[ c ] || jQuery.css(self, c); equals( cur, v, "Make sure that " + c + " is reset (Old: " + v + " Cur: " + cur + ")"); }); + + // manually clean data on modified element + jQuery.removeData(this, 'olddisplay', true); + start(); } @@ -829,9 +841,6 @@ jQuery.makeTest = function( text ){ jQuery("

") .text( text ) .appendTo("#fx-tests") - .click(function(){ - jQuery(this).next().toggle(); - }) .after( elem ); return elem; diff --git a/test/unit/event.js b/test/unit/event.js index cae1a83fe..a6d8cb68b 100644 --- a/test/unit/event.js +++ b/test/unit/event.js @@ -1,4 +1,4 @@ -module("event"); +module("event", { teardown: moduleTeardown }); test("null or undefined handler", function() { expect(2); @@ -80,6 +80,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "focusin"; div.trigger("focusin.a"); + // manually clean up detached elements + div.remove(); + div = jQuery("
").bind("click mouseover", obj, function(e) { equals( e.type, cur, "Verify right multi event was fired." ); equals( e.data, obj, "Make sure the data came in correctly." ); @@ -91,6 +94,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "mouseover"; div.trigger("mouseover"); + // manually clean up detached elements + div.remove(); + div = jQuery("
").bind("focusin.a focusout.b", function(e) { equals( e.type, cur, "Verify right multi event was fired." ); }); @@ -100,6 +106,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "focusout"; div.trigger("focusout.b"); + + // manually clean up detached elements + div.remove(); }); test("bind(), namespace with special add", function() { @@ -525,6 +534,9 @@ test("bind(name, false), unbind(name, false)", function() { jQuery("#ap").unbind("click", false); jQuery("#ap").trigger("click"); equals( main, 1, "Verify that the trigger happened correctly." ); + + // manually clean up events from elements outside the fixture + jQuery("#main").unbind("click"); }); test("bind()/trigger()/unbind() on plain object", function() { @@ -663,13 +675,18 @@ test("hover()", function() { test("trigger() shortcuts", function() { expect(6); - jQuery('
  • Change location
    • ').prependTo('#firstUL').find('a').bind('click', function() { + + var elem = jQuery('
  • Change location
    • ').prependTo('#firstUL'); + elem.find('a').bind('click', function() { var close = jQuery('spanx', this); // same with jQuery(this).find('span'); equals( close.length, 0, "Context element does not exist, length must be zero" ); ok( !close[0], "Context element does not exist, direct access to element must return undefined" ); return false; }).click(); + // manually clean up detached elements + elem.remove(); + jQuery("#check1").click(function() { ok( true, "click event handler for checkbox gets fired twice, see #815" ); }).click(); @@ -688,9 +705,12 @@ test("trigger() shortcuts", function() { jQuery('#simon1').click(); equals( clickCounter, 1, "Check that click, triggers onclick event handler on an a tag also" ); - jQuery('').load(function(){ + elem = jQuery('').load(function(){ ok( true, "Trigger the load event, using the shortcut .load() (#2819)"); }).load(); + + // manually clean up detached elements + elem.remove(); }); test("trigger() bubbling", function() { @@ -725,6 +745,10 @@ test("trigger() bubbling", function() { equals( body, 2, "ap bubble" ); equals( main, 1, "ap bubble" ); equals( ap, 1, "ap bubble" ); + + // manually clean up events from elements outside the fixture + jQuery(document).unbind("click"); + jQuery("html, body, #main").unbind("click"); }); test("trigger(type, [data], [fn])", function() { @@ -768,7 +792,7 @@ test("trigger(type, [data], [fn])", function() { pass = true; try { - jQuery('table:first').bind('test:test', function(){}).trigger('test:test'); + jQuery('#main table:first').bind('test:test', function(){}).trigger('test:test'); } catch (e) { pass = false; } @@ -952,6 +976,9 @@ test("toggle(Function, Function, ...)", function() { var data = jQuery._data( $div[0], 'events' ); ok( !data, "Unbinding one function from toggle unbinds them all"); + // manually clean up detached elements + $div.remove(); + // Test Multi-Toggles var a = [], b = []; $div = jQuery("
    "); @@ -967,6 +994,9 @@ test("toggle(Function, Function, ...)", function() { $div.click(); same( a, [1,2,1], "Check that a click worked with a second toggle, second click." ); same( b, [1,2], "Check that a click worked with a second toggle, second click." ); + + // manually clean up detached elements + $div.remove(); }); test(".live()/.die()", function() { @@ -1277,6 +1307,9 @@ test("live with multiple events", function(){ div.trigger("submit"); equals( count, 2, "Make sure both the click and submit were triggered." ); + + // manually clean up events from elements outside the fixture + div.die(); }); test("live with namespaces", function(){ diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js index e273cf08a..de841445f 100644 --- a/test/unit/manipulation.js +++ b/test/unit/manipulation.js @@ -1,4 +1,4 @@ -module("manipulation"); +module("manipulation", { teardown: moduleTeardown }); // Ensure that an extended Array prototype doesn't break jQuery Array.prototype.arrayProtoFn = function(arg) { throw("arrayProtoFn should not be called"); }; @@ -115,12 +115,19 @@ var testWrap = function(val) { // Wrap an element with a jQuery set and event result = jQuery("
    ").click(function(){ ok(true, "Event triggered."); + + // Remove handlers on detached elements + result.unbind(); + jQuery(this).unbind(); }); j = jQuery("").wrap(result); equals( j[0].parentNode.nodeName.toLowerCase(), "div", "Wrapping works." ); j.parent().trigger("click"); + + // clean up attached elements + QUnit.reset(); } test("wrap(String|Element)", function() { @@ -408,8 +415,12 @@ test("append the same fragment with events (Bug #6997, 5566)", function () { ok(true, "Event exists on original after being unbound on clone"); jQuery(this).unbind('click'); }); - element.clone(true).unbind('click')[0].fireEvent('onclick'); + var clone = element.clone(true).unbind('click'); + clone[0].fireEvent('onclick'); element[0].fireEvent('onclick'); + + // manually clean up detached elements + clone.remove(); } element = jQuery("").click(function () { @@ -894,20 +905,36 @@ test("clone()", function() { ok( true, "Bound event still exists." ); }); - div = div.clone(true).clone(true); + clone = div.clone(true); + + // manually clean up detached elements + div.remove(); + + div = clone.clone(true); + + // manually clean up detached elements + clone.remove(); + equals( div.length, 1, "One element cloned" ); equals( div[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); div.trigger("click"); + // manually clean up detached elements + div.remove(); + div = jQuery("
    ").append([ document.createElement("table"), document.createElement("table") ]); div.find("table").click(function(){ ok( true, "Bound event still exists." ); }); - div = div.clone(true); - equals( div.length, 1, "One element cloned" ); - equals( div[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); - div.find("table:last").trigger("click"); + clone = div.clone(true); + equals( clone.length, 1, "One element cloned" ); + equals( clone[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); + clone.find("table:last").trigger("click"); + + // manually clean up detached elements + div.remove(); + clone.remove(); // this is technically an invalid object, but because of the special // classid instantiation it is the only kind that IE has trouble with, @@ -928,12 +955,16 @@ test("clone()", function() { equals( clone[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); div = jQuery("
    ").data({ a: true }); - var div2 = div.clone(true); - equals( div2.data("a"), true, "Data cloned." ); - div2.data("a", false); - equals( div2.data("a"), false, "Ensure cloned element data object was correctly modified" ); + clone = div.clone(true); + equals( clone.data("a"), true, "Data cloned." ); + clone.data("a", false); + equals( clone.data("a"), false, "Ensure cloned element data object was correctly modified" ); equals( div.data("a"), true, "Ensure cloned element data object is copied, not referenced" ); + // manually clean up detached elements + div.remove(); + clone.remove(); + var form = document.createElement("form"); form.action = "/test/"; var div = document.createElement("div"); @@ -1141,15 +1172,21 @@ var testRemove = function(method) { jQuery("#nonnodes").contents()[method](); equals( jQuery("#nonnodes").contents().length, 0, "Check node,textnode,comment remove works" ); + // manually clean up detached elements + if (method === "detach") { + first.remove(); + } + QUnit.reset(); var count = 0; var first = jQuery("#ap").children(":first"); - var cleanUp = first.click(function() { count++ })[method]().appendTo("body").click(); + var cleanUp = first.click(function() { count++ })[method]().appendTo("#main").click(); equals( method == "remove" ? 0 : 1, count ); - cleanUp.detach(); + // manually clean up detached elements + cleanUp.remove(); }; test("remove()", function() { diff --git a/test/unit/offset.js b/test/unit/offset.js index cfa14449b..329d69f95 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -1,4 +1,4 @@ -module("offset"); +module("offset", { teardown: moduleTeardown }); test("disconnected node", function() { expect(2); diff --git a/test/unit/queue.js b/test/unit/queue.js index eada0eede..31e587db2 100644 --- a/test/unit/queue.js +++ b/test/unit/queue.js @@ -1,4 +1,4 @@ -module("queue"); +module("queue", { teardown: moduleTeardown }); test("queue() with other types",function() { expect(9); diff --git a/test/unit/selector.js b/test/unit/selector.js index 5b758c101..c530c1fdb 100644 --- a/test/unit/selector.js +++ b/test/unit/selector.js @@ -1,4 +1,4 @@ -module("selector"); +module("selector", { teardown: moduleTeardown }); test("element", function() { expect(21); diff --git a/test/unit/traversing.js b/test/unit/traversing.js index 31cffc2eb..f0471d74e 100644 --- a/test/unit/traversing.js +++ b/test/unit/traversing.js @@ -1,4 +1,4 @@ -module("traversing"); +module("traversing", { teardown: moduleTeardown }); test("find(String)", function() { expect(5); From 57cc182a40e909868d41f9b1bb405b06138f6cae Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Mon, 17 Jan 2011 15:22:49 -0600 Subject: [PATCH 5/5] Introduce a temporary hack to allow jQuery.fn.data("events") to continue to work. This will be going away in 1.6. More information will be available in the 1.5 release notes. --- src/data.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/data.js b/src/data.js index a1abc9ed6..21f0e3a55 100644 --- a/src/data.js +++ b/src/data.js @@ -93,6 +93,13 @@ jQuery.extend({ thisCache[ name ] = data; } + // 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; },