fix(jqLite): correctly monkey-patch core jQuery methods

mgol · vojtajina · commit da5f537ccdb0 · 2013-05-23T12:05:55.000-07:00
When real jQuery is present, Angular monkey patch it to fire `$destroy` event.

This commit fixes two issues in the jQuery patch:
- passing a selector to the $.fn.remove method (only fire `$destroy` on the matched elements)
- using `$.fn.html` without parameters as a getter (do not fire `$destroy`)
diff --git a/src/Angular.js b/src/Angular.js
@@ -1040,9 +1040,10 @@ function bindJQuery() {
       injector: JQLitePrototype.injector,
       inheritedData: JQLitePrototype.inheritedData
     });
-    JQLitePatchJQueryRemove('remove', true);
-    JQLitePatchJQueryRemove('empty');
-    JQLitePatchJQueryRemove('html');
+    // Method signature: JQLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments)
+    JQLitePatchJQueryRemove('remove', true, true, false);
+    JQLitePatchJQueryRemove('empty', false, false, false);
+    JQLitePatchJQueryRemove('html', false, false, true);
   } else {
     jqLite = JQLite;
   }
diff --git a/src/jqLite.js b/src/jqLite.js
@@ -107,37 +107,38 @@ function camelCase(name) {
 /////////////////////////////////////////////
 // jQuery mutation patch
 //
-//  In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a
+// In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a
 // $destroy event on all DOM nodes being removed.
 //
 /////////////////////////////////////////////
 
-function JQLitePatchJQueryRemove(name, dispatchThis) {
+function JQLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments) {
   var originalJqFn = jQuery.fn[name];
   originalJqFn = originalJqFn.$original || originalJqFn;
   removePatch.$original = originalJqFn;
   jQuery.fn[name] = removePatch;
 
-  function removePatch() {
-    var list = [this],
+  function removePatch(param) {
+    var list = filterElems && param ? [this.filter(param)] : [this],
         fireEvent = dispatchThis,
         set, setIndex, setLength,
-        element, childIndex, childLength, children,
-        fns, events;
-
-    while(list.length) {
-      set = list.shift();
-      for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) {
-        element = jqLite(set[setIndex]);
-        if (fireEvent) {
-          element.triggerHandler('$destroy');
-        } else {
-          fireEvent = !fireEvent;
-        }
-        for(childIndex = 0, childLength = (children = element.children()).length;
-            childIndex < childLength;
-            childIndex++) {
-          list.push(jQuery(children[childIndex]));
+        element, childIndex, childLength, children;
+
+    if (!getterIfNoArguments || param != null) {
+      while(list.length) {
+        set = list.shift();
+        for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) {
+          element = jqLite(set[setIndex]);
+          if (fireEvent) {
+            element.triggerHandler('$destroy');
+          } else {
+            fireEvent = !fireEvent;
+          }
+          for(childIndex = 0, childLength = (children = element.children()).length;
+              childIndex < childLength;
+              childIndex++) {
+            list.push(jQuery(children[childIndex]));
+          }
         }
       }
     }
diff --git a/test/jQueryPatchSpec.js b/test/jQueryPatchSpec.js
@@ -49,9 +49,55 @@ if (window.jQuery) {
         doc.empty();
       });
 
-      it('should fire on html()', function() {
+      it('should fire on html(param)', function() {
         doc.html('abc');
       });
+
+      it('should fire on html(\'\')', function() {
+        doc.html('');
+      });
+    });
+  });
+
+  describe('jQuery patch eagerness', function() {
+
+    var doc = null;
+    var divSpy = null;
+    var spy1 = null;
+    var spy2 = null;
+
+    beforeEach(function() {
+      divSpy = jasmine.createSpy('div.$destroy');
+      spy1 = jasmine.createSpy('span1.$destroy');
+      spy2 = jasmine.createSpy('span2.$destroy');
+      doc = $('<div><span class=first>abc</span><span class=second>xyz</span></div>');
+      doc.find('span.first').bind('$destroy', spy1);
+      doc.find('span.second').bind('$destroy', spy2);
+    });
+
+    afterEach(function() {
+      expect(divSpy).not.toHaveBeenCalled();
+      expect(spy1).not.toHaveBeenCalled();
+    });
+
+    describe('$detach event is not invoked in too many cases', function() {
+
+      it('should fire only on matched elements on detach(selector)', function() {
+        doc.find('span').detach('.second');
+        expect(spy2).toHaveBeenCalled();
+        expect(spy2.callCount).toEqual(1);
+      });
+
+      it('should fire only on matched elements on remove(selector)', function() {
+        doc.find('span').remove('.second');
+        expect(spy2).toHaveBeenCalled();
+        expect(spy2.callCount).toEqual(1);
+      });
+
+      it('should not fire on html()', function() {
+        doc.html();
+        expect(spy2).not.toHaveBeenCalled();
+      });
     });
   });
 }
