diff --git a/src/ngMessages/messages.js b/src/ngMessages/messages.js
index bbe87bba372a..f79baa18cb71 100644
--- a/src/ngMessages/messages.js
+++ b/src/ngMessages/messages.js
@@ -410,6 +410,13 @@ angular.module('ngMessages', [])
$scope.$watchCollection($attrs.ngMessages || $attrs['for'], ctrl.render);
+ // If the element is destroyed, proactively destroy all the currently visible messages
+ $element.on('$destroy', function() {
+ forEach(messages, function(item) {
+ item.message.detach();
+ });
+ });
+
this.reRender = function() {
if (!renderLater) {
renderLater = true;
@@ -444,6 +451,7 @@ angular.module('ngMessages', [])
function findPreviousMessage(parent, comment) {
var prevNode = comment;
var parentLookup = [];
+
while (prevNode && prevNode !== parent) {
var prevKey = prevNode.$$ngMessageNode;
if (prevKey && prevKey.length) {
@@ -455,8 +463,11 @@ angular.module('ngMessages', [])
if (prevNode.childNodes.length && parentLookup.indexOf(prevNode) == -1) {
parentLookup.push(prevNode);
prevNode = prevNode.childNodes[prevNode.childNodes.length - 1];
+ } else if (prevNode.previousSibling) {
+ prevNode = prevNode.previousSibling;
} else {
- prevNode = prevNode.previousSibling || prevNode.parentNode;
+ prevNode = prevNode.parentNode;
+ parentLookup.push(prevNode);
}
}
}
@@ -669,8 +680,8 @@ function ngMessageDirectiveFactory() {
// when we are destroying the node later.
var $$attachId = currentElement.$$attachId = ngMessagesCtrl.getAttachId();
- // in the event that the parent element is destroyed
- // by any other structural directive then it's time
+ // in the event that the element or a parent element is destroyed
+ // by another structural directive then it's time
// to deregister the message from the controller
currentElement.on('$destroy', function() {
if (currentElement && currentElement.$$attachId === $$attachId) {
diff --git a/test/ngMessages/messagesSpec.js b/test/ngMessages/messagesSpec.js
index 2a03c90075b2..5613844bf591 100644
--- a/test/ngMessages/messagesSpec.js
+++ b/test/ngMessages/messagesSpec.js
@@ -485,6 +485,126 @@ describe('ngMessages', function() {
});
});
+ describe('ngMessage nested nested inside elements', function() {
+
+ it('should not crash or leak memory when the messages are transcluded, the first message is ' +
+ 'visible, and ngMessages is removed by ngIf', function() {
+
+ module(function($compileProvider) {
+ $compileProvider.directive('messageWrap', function() {
+ return {
+ transclude: true,
+ scope: {
+ col: '=col'
+ },
+ template: '
'
+ };
+ });
+ });
+
+ inject(function($rootScope, $compile) {
+
+ element = $compile('' +
+ '
A
' +
+ '
B
' +
+ '
')($rootScope);
+
+ $rootScope.$apply(function() {
+ $rootScope.show = true;
+ $rootScope.col = {
+ a: true,
+ b: true
+ };
+ });
+
+ expect(messageChildren(element).length).toBe(1);
+ expect(trim(element.text())).toEqual('A');
+
+ $rootScope.$apply('show = false');
+
+ expect(messageChildren(element).length).toBe(0);
+ });
+ });
+
+
+ it('should not crash when the first of two nested messages is removed', function() {
+ inject(function($rootScope, $compile) {
+
+ element = $compile(
+ ''
+ )($rootScope);
+
+ $rootScope.$apply(function() {
+ $rootScope.col = {
+ a: true,
+ b: false
+ };
+ });
+
+ expect(messageChildren(element).length).toBe(1);
+ expect(trim(element.text())).toEqual('A');
+
+ var ctrl = element.controller('ngMessages');
+ var deregisterSpy = spyOn(ctrl, 'deregister').and.callThrough();
+
+ var nodeA = element[0].querySelector('[ng-message="a"]');
+ jqLite(nodeA).remove();
+ $rootScope.$digest(); // The next digest triggers the error
+
+ // Make sure removing the element triggers the deregistration in ngMessages
+ expect(trim(deregisterSpy.calls.mostRecent().args[0].nodeValue)).toBe('ngMessage: a');
+ expect(messageChildren(element).length).toBe(0);
+ });
+ });
+
+
+ it('should not crash, but show deeply nested messages correctly after a message ' +
+ 'has been removed', function() {
+ inject(function($rootScope, $compile) {
+
+ element = $compile(
+ '' +
+ '
' +
+ '
A
' +
+ '
' +
+ '
D
' +
+ '
' +
+ '
'
+ )($rootScope);
+
+ $rootScope.$apply(function() {
+ $rootScope.col = {
+ a: true,
+ b: true
+ };
+ });
+
+ expect(messageChildren(element).length).toBe(2);
+ expect(trim(element.text())).toEqual('AB');
+
+ var ctrl = element.controller('ngMessages');
+ var deregisterSpy = spyOn(ctrl, 'deregister').and.callThrough();
+
+ var nodeB = element[0].querySelector('[ng-message="b"]');
+ jqLite(nodeB).remove();
+ $rootScope.$digest(); // The next digest triggers the error
+
+ // Make sure removing the element triggers the deregistration in ngMessages
+ expect(trim(deregisterSpy.calls.mostRecent().args[0].nodeValue)).toBe('ngMessage: b');
+ expect(messageChildren(element).length).toBe(1);
+ expect(trim(element.text())).toEqual('A');
+ });
+ });
+ });
+
describe('when including templates', function() {
they('should work with a dynamic collection model which is managed by ngRepeat',
{'': '
' +
@@ -691,6 +811,37 @@ describe('ngMessages', function() {
expect(trim(element.text())).toEqual("C");
}));
+
+ it('should properly detect a previous message, even if it was registered later',
+ inject(function($compile, $rootScope, $templateCache) {
+ $templateCache.put('include.html', '
A
');
+ var html =
+ '
' +
+ '
' +
+ '
B
' +
+ '
C
' +
+ '
';
+
+ element = $compile(html)($rootScope);
+ $rootScope.$apply('items = {b: true, c: true}');
+
+ expect(element.text()).toBe('B');
+
+ var ctrl = element.controller('ngMessages');
+ var deregisterSpy = spyOn(ctrl, 'deregister').and.callThrough();
+
+ var nodeB = element[0].querySelector('[ng-message="b"]');
+ jqLite(nodeB).remove();
+
+ // Make sure removing the element triggers the deregistration in ngMessages
+ expect(trim(deregisterSpy.calls.mostRecent().args[0].nodeValue)).toBe('ngMessage: b');
+
+ $rootScope.$apply('items.a = true');
+
+ expect(element.text()).toBe('A');
+ })
+ );
+
});
describe('when multiple', function() {