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 gh-5214
Closes gh-5523

Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com>
Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

2 files changed, 44 insertions, 1 deletions

diff --git a/src/manipulation/getAll.js b/src/manipulation/getAll.js
index db876202d..86526a256 100644
--- a/src/manipulation/getAll.js
+++ b/src/manipulation/getAll.js
@@ -1,5 +1,6 @@
 import { jQuery } from "../core.js";
 import { nodeName } from "../core/nodeName.js";
+import { arr } from "../var/arr.js";
 
 export function getAll( context, tag ) {
 
@@ -8,7 +9,9 @@ export function getAll( context, tag ) {
 	var ret;
 
 	if ( typeof context.getElementsByTagName !== "undefined" ) {
-		ret = context.getElementsByTagName( tag || "*" );
+
+		// Use slice to snapshot the live collection from gEBTN
+		ret = arr.slice.call( context.getElementsByTagName( tag || "*" ) );
 
 	} else if ( typeof context.querySelectorAll !== "undefined" ) {
 		ret = context.querySelectorAll( tag || "*" );
diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index ff1995cb1..8300b4b9c 100644
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -3099,3 +3099,43 @@ testIframe(
 		} );
 	}
 );
+
+QUnit.test( "should handle node removal in event's remove hook (gh-5214)", function( assert ) {
+
+	assert.expect( 4 );
+
+	jQuery(
+		"<div id='container'>" +
+		"	<div class='guarded removeself' data-elt='one'>" +
+		"		Guarded 1" +
+		"	</div>" +
+		"	<div class='guarded' data-elt='two'>" +
+		"		Guarded 2" +
+		"	</div>" +
+		"	<div class='guarded' data-elt='three'>" +
+		"		Guarded 3" +
+		"	</div>" +
+		"</div>"
+	).appendTo( "#qunit-fixture" );
+
+	// Define the custom event handler
+	jQuery.event.special.removeondestroy = {
+		remove: function( ) {
+			var $t = jQuery( this );
+			assert.step( $t.data( "elt" ) );
+			if ( $t.is( ".removeself" ) ) {
+				$t.remove();
+			}
+		}
+	};
+
+	// Attach an empty handler to trigger the `remove`
+	// logic for the custom event when the element is removed.
+	jQuery( ".guarded" ).on( "removeondestroy", function( ) { } );
+
+	// Trigger the event's removal logic by emptying the container
+	jQuery( "#container" ).empty();
+
+	assert.verifySteps( [ "one", "two", "three" ], "All elements were processed in order" );
+} );
+