Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 815053e

Browse files
mgolvojtajina
authored andcommitted
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`)
1 parent 6173abe commit 815053e

File tree

3 files changed

+72
-24
lines changed

3 files changed

+72
-24
lines changed

src/Angular.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -1016,9 +1016,10 @@ function bindJQuery() {
10161016
injector: JQLitePrototype.injector,
10171017
inheritedData: JQLitePrototype.inheritedData
10181018
});
1019-
JQLitePatchJQueryRemove('remove', true);
1020-
JQLitePatchJQueryRemove('empty');
1021-
JQLitePatchJQueryRemove('html');
1019+
// Method signature: JQLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments)
1020+
JQLitePatchJQueryRemove('remove', true, true, false);
1021+
JQLitePatchJQueryRemove('empty', false, false, false);
1022+
JQLitePatchJQueryRemove('html', false, false, true);
10221023
} else {
10231024
jqLite = JQLite;
10241025
}

src/jqLite.js

+21-20
Original file line numberDiff line numberDiff line change
@@ -107,37 +107,38 @@ function camelCase(name) {
107107
/////////////////////////////////////////////
108108
// jQuery mutation patch
109109
//
110-
// In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a
110+
// In conjunction with bindJQuery intercepts all jQuery's DOM destruction apis and fires a
111111
// $destroy event on all DOM nodes being removed.
112112
//
113113
/////////////////////////////////////////////
114114

115-
function JQLitePatchJQueryRemove(name, dispatchThis) {
115+
function JQLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments) {
116116
var originalJqFn = jQuery.fn[name];
117117
originalJqFn = originalJqFn.$original || originalJqFn;
118118
removePatch.$original = originalJqFn;
119119
jQuery.fn[name] = removePatch;
120120

121-
function removePatch() {
122-
var list = [this],
121+
function removePatch(param) {
122+
var list = filterElems && param ? [this.filter(param)] : [this],
123123
fireEvent = dispatchThis,
124124
set, setIndex, setLength,
125-
element, childIndex, childLength, children,
126-
fns, events;
127-
128-
while(list.length) {
129-
set = list.shift();
130-
for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) {
131-
element = jqLite(set[setIndex]);
132-
if (fireEvent) {
133-
element.triggerHandler('$destroy');
134-
} else {
135-
fireEvent = !fireEvent;
136-
}
137-
for(childIndex = 0, childLength = (children = element.children()).length;
138-
childIndex < childLength;
139-
childIndex++) {
140-
list.push(jQuery(children[childIndex]));
125+
element, childIndex, childLength, children;
126+
127+
if (!getterIfNoArguments || param != null) {
128+
while(list.length) {
129+
set = list.shift();
130+
for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) {
131+
element = jqLite(set[setIndex]);
132+
if (fireEvent) {
133+
element.triggerHandler('$destroy');
134+
} else {
135+
fireEvent = !fireEvent;
136+
}
137+
for(childIndex = 0, childLength = (children = element.children()).length;
138+
childIndex < childLength;
139+
childIndex++) {
140+
list.push(jQuery(children[childIndex]));
141+
}
141142
}
142143
}
143144
}

test/jQueryPatchSpec.js

+47-1
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,55 @@ if (window.jQuery) {
4949
doc.empty();
5050
});
5151

52-
it('should fire on html()', function() {
52+
it('should fire on html(param)', function() {
5353
doc.html('abc');
5454
});
55+
56+
it('should fire on html(\'\')', function() {
57+
doc.html('');
58+
});
59+
});
60+
});
61+
62+
describe('jQuery patch eagerness', function() {
63+
64+
var doc = null;
65+
var divSpy = null;
66+
var spy1 = null;
67+
var spy2 = null;
68+
69+
beforeEach(function() {
70+
divSpy = jasmine.createSpy('div.$destroy');
71+
spy1 = jasmine.createSpy('span1.$destroy');
72+
spy2 = jasmine.createSpy('span2.$destroy');
73+
doc = $('<div><span class=first>abc</span><span class=second>xyz</span></div>');
74+
doc.find('span.first').bind('$destroy', spy1);
75+
doc.find('span.second').bind('$destroy', spy2);
76+
});
77+
78+
afterEach(function() {
79+
expect(divSpy).not.toHaveBeenCalled();
80+
expect(spy1).not.toHaveBeenCalled();
81+
});
82+
83+
describe('$detach event is not invoked in too many cases', function() {
84+
85+
it('should fire only on matched elements on detach(selector)', function() {
86+
doc.find('span').detach('.second');
87+
expect(spy2).toHaveBeenCalled();
88+
expect(spy2.callCount).toEqual(1);
89+
});
90+
91+
it('should fire only on matched elements on remove(selector)', function() {
92+
doc.find('span').remove('.second');
93+
expect(spy2).toHaveBeenCalled();
94+
expect(spy2.callCount).toEqual(1);
95+
});
96+
97+
it('should not fire on html()', function() {
98+
doc.html();
99+
expect(spy2).not.toHaveBeenCalled();
100+
});
55101
});
56102
});
57103
}

0 commit comments

Comments
 (0)