Permalink
Browse files

Data: remove the expando when there's no more data

Fixes gh-1760
Close gh-2271
  • Loading branch information...
timmywil committed May 6, 2015
1 parent 764dc94 commit 56bb677725b21415905e5c3eeb1e05be4480e780
Showing with 48 additions and 13 deletions.
  1. +6 −4 src/data/Data.js
  2. +2 −7 src/event.js
  3. +19 −2 test/unit/data.js
  4. +21 −0 test/unit/event.js
View
@@ -120,10 +120,7 @@ Data.prototype = {
return;
}
if ( key === undefined ) {
this.register( owner );
} else {
if ( key !== undefined ) {
// Support array or space separated string of keys
if ( jQuery.isArray( key ) ) {
@@ -147,6 +144,11 @@ Data.prototype = {
delete cache[ key[ i ] ];
}
}
// Remove the expando if there's no more data
if ( key === undefined || jQuery.isEmptyObject( cache ) ) {
delete owner[ this.expando ];

This comment has been minimized.

Show comment
Hide comment
@jbedard

jbedard Jun 6, 2015

Contributor

@timmywil I think this needs to be owner[ this.expando ] = undefined. See #1664

@jbedard

jbedard Jun 6, 2015

Contributor

@timmywil I think this needs to be owner[ this.expando ] = undefined. See #1664

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jun 7, 2015

Member

Hmm, except we no longer set to undefined in cleanData either. I wonder if that is still a problem in Chrome.

@timmywil

timmywil Jun 7, 2015

Member

Hmm, except we no longer set to undefined in cleanData either. I wonder if that is still a problem in Chrome.

This comment has been minimized.

Show comment
Hide comment
@jbedard

jbedard Jun 7, 2015

Contributor

The chrome bug seems to no longer be accessible for some reason, but http://jsperf.com/cleandata/4 still shows chrome 43 is much slower when using delete. In chrome 45 delete is catching up but still seems slower.

@jbedard

jbedard Jun 7, 2015

Contributor

The chrome bug seems to no longer be accessible for some reason, but http://jsperf.com/cleandata/4 still shows chrome 43 is much slower when using delete. In chrome 45 delete is catching up but still seems slower.

}
},
hasData: function( owner ) {
var cache = owner[ this.expando ];
View
@@ -216,14 +216,9 @@ jQuery.event = {
}
}
// Remove the expando if it's no longer used
// Remove data and the expando if it's no longer used
if ( jQuery.isEmptyObject( events ) ) {
// Normally this should go through the data api
// but since event.js owns these properties,
// this exception is made for the sake of optimizing
// the operation.
delete elemData.handle;
delete elemData.events;
dataPriv.remove( elem, "handle events" );
}
},
View
@@ -837,7 +837,24 @@ test("Check proper data removal of non-element descendants nodes (#8335)", 1, fu
});
testIframeWithCallback( "enumerate data attrs on body (#14894)", "data/dataAttrs.html", function( result ) {
expect(1);
expect( 1 );
equal( result, "ok", "enumeration of data- attrs on body" );
});
test( "Check that the expando is removed when there's no more data", function() {
expect( 1 );
equal(result, "ok", "enumeration of data- attrs on body" );
var key,
div = jQuery( "<div/>" );
div.data( "some", "data" );
equal( div.data( "some" ), "data", "Data is added" );
div.removeData( "some" );
// Make sure the expando is gone
for ( key in div[ 0 ] ) {
if ( /^jQuery/.test( key ) ) {
ok( false, "Expando was not removed when there was no more data" );
}
}
});
View
@@ -2657,6 +2657,27 @@ test( "Inline event result is returned (#13993)", function() {
equal( result, 42, "inline handler returned value" );
});
test( ".off() removes the expando when there's no more data", function() {
expect( 1 );
var key,
div = jQuery( "<div/>" ).appendTo( "#qunit-fixture" );
div.on( "click", false );
div.on( "custom", function() {
ok( true, "Custom event triggered" );
} );
div.trigger( "custom" );
div.off( "click custom" );
// Make sure the expando is gone
for ( key in div[ 0 ] ) {
if ( /^jQuery/.test( key ) ) {
ok( false, "Expando was not removed when there was no more data" );
}
}
});
// This tests are unreliable in Firefox
if ( !(/firefox/i.test( window.navigator.userAgent )) ) {
test( "Check order of focusin/focusout events", 2, function() {

0 comments on commit 56bb677

Please sign in to comment.