diff --git a/src/ngMessages/messages.js b/src/ngMessages/messages.js index b6fbf622cd19..445122b67102 100644 --- a/src/ngMessages/messages.js +++ b/src/ngMessages/messages.js @@ -417,13 +417,6 @@ angular.module('ngMessages', [], function initAngularHelpers() { $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; @@ -498,6 +491,9 @@ angular.module('ngMessages', [], function initAngularHelpers() { function removeMessageNode(parent, comment, key) { var messageNode = messages[key]; + // This message node may have already been removed by a call to deregister() + if (!messageNode) return; + var match = findPreviousMessage(parent, comment); if (match) { match.next = messageNode.next; @@ -702,6 +698,8 @@ function ngMessageDirectiveFactory() { // by another structural directive then it's time // to deregister the message from the controller currentElement.on('$destroy', function() { + // If the message element was removed via a call to `detach` then `currentElement` will be null + // So this handler only handles cases where something else removed the message element. if (currentElement && currentElement.$$attachId === $$attachId) { ngMessagesCtrl.deregister(commentNode); messageCtrl.detach(); @@ -719,6 +717,14 @@ function ngMessageDirectiveFactory() { } } }); + + // We need to ensure that this directive deregisters itself when it no longer exists + // Normally this is done when the attached element is destroyed; but if this directive + // gets removed before we attach the message to the DOM there is nothing to watch + // in which case we must deregister when the containing scope is destroyed. + scope.$on('$destroy', function() { + ngMessagesCtrl.deregister(commentNode); + }); } }; }]; diff --git a/test/ngMessages/messagesSpec.js b/test/ngMessages/messagesSpec.js index bf060ed65c8c..b86f764f37d0 100644 --- a/test/ngMessages/messagesSpec.js +++ b/test/ngMessages/messagesSpec.js @@ -636,6 +636,30 @@ describe('ngMessages', function() { }) ); + it('should unregister the ngMessage even if it was never attached', + inject(function($compile, $rootScope) { + var html = + '
' + + '
ERROR
' + + '
'; + + element = $compile(html)($rootScope); + + var ctrl = element.controller('ngMessages'); + + expect(messageChildren(element).length).toBe(0); + expect(Object.keys(ctrl.messages).length).toEqual(0); + + $rootScope.$apply('show = true'); + expect(messageChildren(element).length).toBe(0); + expect(Object.keys(ctrl.messages).length).toEqual(1); + + $rootScope.$apply('show = false'); + expect(messageChildren(element).length).toBe(0); + expect(Object.keys(ctrl.messages).length).toEqual(0); + }) + ); + describe('when including templates', function() { they('should work with a dynamic collection model which is managed by ngRepeat',