diff options
| author | Michał Gołębiowski | 2013-04-30 11:29:07 +0200 | 
|---|---|---|
| committer | Vojta Jina | 2013-05-23 12:05:55 -0700 | 
| commit | da5f537ccdb0a7b4155f13f7a70ca7981ad6f689 (patch) | |
| tree | 30fc4349dc22289d110f1bfdea0db076121a89c0 | |
| parent | 041f118b01933fa07098f1333be8e2cf1a28f248 (diff) | |
| download | angular.js-da5f537ccdb0a7b4155f13f7a70ca7981ad6f689.tar.bz2 | |
fix(jqLite): correctly monkey-patch core jQuery methods
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`)
| -rw-r--r-- | src/Angular.js | 7 | ||||
| -rw-r--r-- | src/jqLite.js | 41 | ||||
| -rw-r--r-- | test/jQueryPatchSpec.js | 48 | 
3 files changed, 72 insertions, 24 deletions
| diff --git a/src/Angular.js b/src/Angular.js index 7bbee592..aefe7ad5 100644 --- 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 index 6809da74..218b9683 100644 --- 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 index 19f4d821..0680a0c3 100644 --- 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(); +      });      });    });  } | 
