Skip to content
Permalink
Browse files

Widget: Hook into jQuery.cleanData to auto-destroy widgets. Fixes #60…

…08 - Widget: auto-destroy is broken in jQuery 1.4.
  • Loading branch information...
scottgonzalez committed Sep 3, 2010
1 parent e5f3bfc commit 0a0a39f896f83412dc91bedd6819c3a3a0932302
Showing with 93 additions and 14 deletions.
  1. +3 −1 tests/unit/widget/widget.html
  2. +67 −0 tests/unit/widget/widget_core.js
  3. +23 −13 ui/jquery.ui.widget.js
@@ -29,7 +29,9 @@ <h2 id="qunit-userAgent"></h2>
<div id="main" style="position: absolute; top: -10000px; left: -10000px;">

<div id="widget-wrapper">
<div id="widget"></div>
<div id="widget">
<div>...</div>
</div>
</div>

</div>
@@ -409,4 +409,71 @@ test( "._trigger() - provide event and ui", function() {
.testWidget( "testEvent" );
});

test( "auto-destroy - .remove()", function() {
expect( 1 );
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( true, "destroyed from .remove()" );
}
});
$( "#widget" ).testWidget().remove();
});

test( "auto-destroy - .remove() on parent", function() {
expect( 1 );
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( true, "destroyed from .remove() on parent" );
}
});
$( "#widget" ).testWidget().parent().remove();
});

test( "auto-destroy - .remove() on child", function() {
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( false, "destroyed from .remove() on child" );
}
});
$( "#widget" ).testWidget().children().remove();
// http://github.com/jquery/qunit/pull/34
$.ui.testWidget.prototype.destroy = $.noop;
});

test( "auto-destroy - .empty()", function() {
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( false, "destroyed from .empty()" );
}
});
$( "#widget" ).testWidget().empty();
// http://github.com/jquery/qunit/pull/34
$.ui.testWidget.prototype.destroy = $.noop;
});

test( "auto-destroy - .empty() on parent", function() {
expect( 1 );
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( true, "destroyed from .empty() on parent" );
}
});
$( "#widget" ).testWidget().parent().empty();
});

test( "auto-destroy - .detach()", function() {
$.widget( "ui.testWidget", {
_create: function() {},
destroy: function() {
ok( false, "destroyed from .detach()" );
}
});
$( "#widget" ).testWidget().detach();
});

})( jQuery );
@@ -9,20 +9,30 @@
*/
(function( $, undefined ) {

var _remove = $.fn.remove;

$.fn.remove = function( selector, keepData ) {
return this.each(function() {
if ( !keepData ) {
if ( !selector || $.filter( selector, [ this ] ).length ) {
$( "*", this ).add( [ this ] ).each(function() {
$( this ).triggerHandler( "remove" );
});
}
// jQuery 1.4+
if ( $.cleanData ) {
var _cleanData = $.cleanData;
$.cleanData = function( elems ) {
for ( var i = 0, elem; (elem = elems[i]) != null; i++ ) {
$( elem ).triggerHandler( "remove" );
}
return _remove.call( $(this), selector, keepData );
});
};
_cleanData( elems );

This comment has been minimized.

Copy link
@boraMan

boraMan Jan 17, 2013

There's an additional (internal) argument 'acceptData' in jQuery.cleanData - why's that not passed here?
(seems to be a perf.opt. as it just prevents some duplicate calls to jQuery.acceptData)

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Jan 17, 2013

Author Member

Notice the word internal that you used? Also, it doesn't exist in all versions.

This comment has been minimized.

Copy link
@boraMan

boraMan Jan 17, 2013

Absolutely - but doesn't that also apply to the whole .cleanData itself?
(Did you already think about using $.event.special instead of hooking into cleanData and changing some of its behavior?)

This comment has been minimized.

Copy link
@scottgonzalez

scottgonzalez Jan 17, 2013

Author Member

Think about it? yes? Actually try it? no. Would you like to send a pull request along with performance tests?

This comment has been minimized.

Copy link
@boraMan

boraMan Jan 17, 2013

Ok, I'll give it a try. Could you give me a hint how to submit performance tests? (sorry - I'm new in here...)

};
} else {
var _remove = $.fn.remove;
$.fn.remove = function( selector, keepData ) {
return this.each(function() {
if ( !keepData ) {
if ( !selector || $.filter( selector, [ this ] ).length ) {
$( "*", this ).add( [ this ] ).each(function() {
$( this ).triggerHandler( "remove" );
});
}
}
return _remove.call( $(this), selector, keepData );
});
};
}

$.widget = function( name, base, prototype ) {
var namespace = name.split( "." )[ 0 ],

2 comments on commit 0a0a39f

@ehynds

This comment has been minimized.

Copy link
Contributor

replied Sep 3, 2010

Why destroy widgets on detach?

@scottgonzalez

This comment has been minimized.

Copy link
Member Author

replied Sep 3, 2010

We don't, the test makes sure that destroy isn't called on .detach().

Please sign in to comment.
You can’t perform that action at this time.