fix(jQuery): cooperate with other libraries monkey-patching jQuery.cleanData

Some libraries (like jQuery UI) patch jQuery.cleanData as well. This commit
makes Angular work correctly even if such external patching was done after
the Angular one.

Fixes #8471

Author: mgol

src/.jshintrc

@@ -88,6 +88,8 @@
     "getBlockElements": false,
     "VALIDITY_STATE_PROPERTY": false,
 
+    "skipDestroyOnNextJQueryCleanData": true,
+
     /* filters.js */
     "getFirstThursdayOfYear": false,
 

src/Angular.js

@@ -1454,8 +1454,15 @@ function snake_case(name, separator) {
   });
 }
 
+var bindJQueryFired = false;
+var skipDestroyOnNextJQueryCleanData;
 function bindJQuery() {
   var originalCleanData;
+
+  if (bindJQueryFired) {
+    return;
+  }
+
   // bind to jQuery if present;
   jQuery = window.jQuery;
   // Use jQuery if it exists with proper functionality, otherwise default to us.
@@ -1472,25 +1479,28 @@ function bindJQuery() {
       inheritedData: JQLitePrototype.inheritedData
     });
 
-    originalCleanData = jQuery.cleanData;
-    // Prevent double-proxying.
-    originalCleanData = originalCleanData.$$original || originalCleanData;
-
     // All nodes removed from the DOM via various jQuery APIs like .remove()
     // are passed through jQuery.cleanData. Monkey-patch this method to fire
     // the $destroy event on all removed nodes.
+    originalCleanData = jQuery.cleanData;
     jQuery.cleanData = function(elems) {
-      for (var i = 0, elem; (elem = elems[i]) != null; i++) {
-        jQuery(elem).triggerHandler('$destroy');
+      if (!skipDestroyOnNextJQueryCleanData) {
+        for (var i = 0, elem; (elem = elems[i]) != null; i++) {
+          jQuery(elem).triggerHandler('$destroy');
+        }
+      } else {
+        skipDestroyOnNextJQueryCleanData = false;
       }
       originalCleanData(elems);
     };
-    jQuery.cleanData.$$original = originalCleanData;
   } else {
     jqLite = JQLite;
   }
 
   angular.element = jqLite;
+
+  // Prevent double-proxying.
+  bindJQueryFired = true;
 }
 
 /**

src/ng/compile.js

@@ -2043,11 +2043,15 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
       if (!jQuery) {
         delete jqLite.cache[firstElementToRemove[jqLite.expando]];
       } else {
-        // jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after the replaced
-        // element. Note that we need to use the original method here and not the one monkey-patched by Angular
-        // since the patched method emits the $destroy event causing the scope to be trashed and we do need
-        // the very same scope to work with the new element.
-        jQuery.cleanData.$$original([firstElementToRemove]);
+        // jQuery 2.x doesn't expose the data storage. Use jQuery.cleanData to clean up after
+        // the replaced element. The cleanData version monkey-patched by Angular would cause
+        // the scope to be trashed and we do need the very same scope to work with the new
+        // element. However, we cannot just cache the non-patched version and use it here as
+        // that would break if another library patches the method after Angular does (one
+        // example is jQuery UI). Instead, set a flag indicating scope destroying should be
+        // skipped this one time.
+        skipDestroyOnNextJQueryCleanData = true;
+        jQuery.cleanData([firstElementToRemove]);
       }
 
       for (var k = 1, kk = elementsToRemove.length; k < kk; k++) {

test/helpers/testabilityPatch.js

@@ -112,6 +112,11 @@ function dealoc(obj) {
 
   function cleanup(element) {
     element.off().removeData();
+    if (window.jQuery) {
+      // jQuery 2.x doesn't expose the cache storage; ensure all element data
+      // is removed during its cleanup.
+      jQuery.cleanData([element]);
+    }
     // Note:  We aren't using element.contents() here.  Under jQuery, element.contents() can fail
     // for IFRAME elements.  jQuery explicitly uses (element.contentDocument ||
     // element.contentWindow.document) and both properties are null for IFRAMES that aren't attached

test/ng/compileSpec.js

@@ -4118,26 +4118,57 @@ describe('$compile', function() {
       });
 
       if (jQuery) {
-        it('should clean up after a replaced element', inject(function ($compile) {
-          var privateData, firstRepeatedElem;
+        describe('cleaning up after a replaced element', function () {
+          var $compile, xs;
+          beforeEach(inject(function (_$compile_) {
+            $compile = _$compile_;
+            xs = [0, 1];
+          }));
 
-          element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);
+          function testCleanup() {
+            var privateData, firstRepeatedElem;
 
-          $rootScope.$apply('xs = [0,1]');
-          firstRepeatedElem = element.children('.ng-scope').eq(0);
+            element = $compile('<div><div ng-repeat="x in xs">{{x}}</div></div>')($rootScope);
 
-          expect(firstRepeatedElem.data('$scope')).toBeDefined();
-          privateData = jQuery._data(firstRepeatedElem[0]);
-          expect(privateData.events).toBeDefined();
-          expect(privateData.events.$destroy).toBeDefined();
-          expect(privateData.events.$destroy[0]).toBeDefined();
+            $rootScope.$apply('xs = [' + xs + ']');
+            firstRepeatedElem = element.children('.ng-scope').eq(0);
 
-          $rootScope.$apply('xs = null');
+            expect(firstRepeatedElem.data('$scope')).toBeDefined();
+            privateData = jQuery._data(firstRepeatedElem[0]);
+            expect(privateData.events).toBeDefined();
+            expect(privateData.events.$destroy).toBeDefined();
+            expect(privateData.events.$destroy[0]).toBeDefined();
 
-          expect(firstRepeatedElem.data('$scope')).not.toBeDefined();
-          privateData = jQuery._data(firstRepeatedElem[0]);
-          expect(privateData && privateData.events).not.toBeDefined();
-        }));
+            $rootScope.$apply('xs = null');
+
+            expect(firstRepeatedElem.data('$scope')).not.toBeDefined();
+            privateData = jQuery._data(firstRepeatedElem[0]);
+            expect(privateData && privateData.events).not.toBeDefined();
+          }
+
+          it('should work without external libraries (except jQuery)', testCleanup);
+
+          it('should work with another library patching jQuery.cleanData after Angular', function () {
+            var cleanedCount = 0;
+            var currentCleanData = jQuery.cleanData;
+            jQuery.cleanData = function (elems) {
+              cleanedCount += elems.length;
+              // Don't return the output and expicitly pass only the first parameter
+              // so that we're sure we're not relying on either of them. jQuery UI patch
+              // behaves in this way.
+              currentCleanData(elems);
+            };
+
+            testCleanup();
+
+            // The initial ng-repeat div is dumped after parsing hence we expect cleanData
+            // count to be one larger than size of the iterated array.
+            expect(cleanedCount).toBe(xs.length + 1);
+
+            // Restore the previous jQuery.cleanData.
+            jQuery.cleanData = currentCleanData;
+          });
+        });
       }