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>
(cherry picked from commit 3cad5c435aa2333c39baa55a8bceb2b6bf1e2721)

diff --git a/src/manipulation/getAll.js b/src/manipulation/getAll.js
index 65dcc1e90bf4f6ee0e19b8a3c8433b22d747c060..c175dc542a53b0293fa6d7081ecc2316b188b023 100644 (file)
--- a/src/manipulation/getAll.js
+++ b/src/manipulation/getAll.js
@@ -1,7 +1,8 @@
 define( [
        "../core",
-       "../core/nodeName"
-], function( jQuery, nodeName ) {
+       "../core/nodeName",
+       "../var/arr"
+], function( jQuery, nodeName, arr ) {
 
 "use strict";
 
@@ -12,7 +13,9 @@ 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 7b52396d9f5a97aeec8961e8440aadfd6359f59b..953b2ca5cea3421a4f2bb985d113201217a8601a 100644 (file)
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -3110,3 +3110,43 @@ QUnit.test( "Sanitized HTML doesn't get unsanitized", function( assert ) {
                test( "<noembed><noembed/><img src=url404 onerror=xss(12)>" );
        }
 } );
+
+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" );
+} );
+