Skip to content

Commit 3cad5c4

Browse files
ac-mmimgolgibson042
authored
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>
1 parent 6d78c07 commit 3cad5c4

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

src/manipulation/getAll.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { jQuery } from "../core.js";
22
import { nodeName } from "../core/nodeName.js";
3+
import { arr } from "../var/arr.js";
34

45
export function getAll( context, tag ) {
56

@@ -8,7 +9,9 @@ export function getAll( context, tag ) {
89
var ret;
910

1011
if ( typeof context.getElementsByTagName !== "undefined" ) {
11-
ret = context.getElementsByTagName( tag || "*" );
12+
13+
// Use slice to snapshot the live collection from gEBTN
14+
ret = arr.slice.call( context.getElementsByTagName( tag || "*" ) );
1215

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

test/unit/manipulation.js

+40
Original file line numberDiff line numberDiff line change
@@ -3099,3 +3099,43 @@ testIframe(
30993099
} );
31003100
}
31013101
);
3102+
3103+
QUnit.test( "should handle node removal in event's remove hook (gh-5214)", function( assert ) {
3104+
3105+
assert.expect( 4 );
3106+
3107+
jQuery(
3108+
"<div id='container'>" +
3109+
" <div class='guarded removeself' data-elt='one'>" +
3110+
" Guarded 1" +
3111+
" </div>" +
3112+
" <div class='guarded' data-elt='two'>" +
3113+
" Guarded 2" +
3114+
" </div>" +
3115+
" <div class='guarded' data-elt='three'>" +
3116+
" Guarded 3" +
3117+
" </div>" +
3118+
"</div>"
3119+
).appendTo( "#qunit-fixture" );
3120+
3121+
// Define the custom event handler
3122+
jQuery.event.special.removeondestroy = {
3123+
remove: function( ) {
3124+
var $t = jQuery( this );
3125+
assert.step( $t.data( "elt" ) );
3126+
if ( $t.is( ".removeself" ) ) {
3127+
$t.remove();
3128+
}
3129+
}
3130+
};
3131+
3132+
// Attach an empty handler to trigger the `remove`
3133+
// logic for the custom event when the element is removed.
3134+
jQuery( ".guarded" ).on( "removeondestroy", function( ) { } );
3135+
3136+
// Trigger the event's removal logic by emptying the container
3137+
jQuery( "#container" ).empty();
3138+
3139+
assert.verifySteps( [ "one", "two", "three" ], "All elements were processed in order" );
3140+
} );
3141+

0 commit comments

Comments
 (0)