Skip to content

Commit 752e911

Browse files
ac-mmimgol
authored andcommitted
Manipulation: Make jQuery.cleanData not skip elements during cleanup
When passing a result of `getElementByTagsName` to `jQuery.cleanData`, convert it to an array first. Otherwise, a live NodeList is passed and if any of the event cleanups remove the element itself, a collection is modified during the iteration, making `jQuery.cleanData` skip cleanup for some elements. Fixes jquerygh-5214 Closes jquerygh-5523 Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com> Co-authored-by: Richard Gibson <richard.gibson@gmail.com> (cherry picked from commit 3cad5c4)
1 parent fb281ca commit 752e911

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

src/manipulation/getAll.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
define( [
22
"../core",
3-
"../core/nodeName"
4-
], function( jQuery, nodeName ) {
3+
"../core/nodeName",
4+
"../var/arr"
5+
], function( jQuery, nodeName, arr ) {
56

67
"use strict";
78

@@ -12,7 +13,9 @@ function getAll( context, tag ) {
1213
var ret;
1314

1415
if ( typeof context.getElementsByTagName !== "undefined" ) {
15-
ret = context.getElementsByTagName( tag || "*" );
16+
17+
// Use slice to snapshot the live collection from gEBTN
18+
ret = arr.slice.call( context.getElementsByTagName( tag || "*" ) );
1619

1720
} else if ( typeof context.querySelectorAll !== "undefined" ) {
1821
ret = context.querySelectorAll( tag || "*" );

test/unit/manipulation.js

+40
Original file line numberDiff line numberDiff line change
@@ -3110,3 +3110,43 @@ QUnit.test( "Sanitized HTML doesn't get unsanitized", function( assert ) {
31103110
test( "<noembed><noembed/><img src=url404 onerror=xss(12)>" );
31113111
}
31123112
} );
3113+
3114+
QUnit.test( "should handle node removal in event's remove hook (gh-5214)", function( assert ) {
3115+
3116+
assert.expect( 4 );
3117+
3118+
jQuery(
3119+
"<div id='container'>" +
3120+
" <div class='guarded removeself' data-elt='one'>" +
3121+
" Guarded 1" +
3122+
" </div>" +
3123+
" <div class='guarded' data-elt='two'>" +
3124+
" Guarded 2" +
3125+
" </div>" +
3126+
" <div class='guarded' data-elt='three'>" +
3127+
" Guarded 3" +
3128+
" </div>" +
3129+
"</div>"
3130+
).appendTo( "#qunit-fixture" );
3131+
3132+
// Define the custom event handler
3133+
jQuery.event.special.removeondestroy = {
3134+
remove: function( ) {
3135+
var $t = jQuery( this );
3136+
assert.step( $t.data( "elt" ) );
3137+
if ( $t.is( ".removeself" ) ) {
3138+
$t.remove();
3139+
}
3140+
}
3141+
};
3142+
3143+
// Attach an empty handler to trigger the `remove`
3144+
// logic for the custom event when the element is removed.
3145+
jQuery( ".guarded" ).on( "removeondestroy", function( ) { } );
3146+
3147+
// Trigger the event's removal logic by emptying the container
3148+
jQuery( "#container" ).empty();
3149+
3150+
assert.verifySteps( [ "one", "two", "three" ], "All elements were processed in order" );
3151+
} );
3152+

0 commit comments

Comments
 (0)