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

Commit 1441560

Browse files
committed
fix(ngMessages): create new scope for ngMessage, clean it up correctly
Previously, ngMessage elements used the same scope as ngMessages. When ngMessage has interpolation in the textContent, then removing the message would not remove the watcher from the scope - it would only be removed when the whole ngMessages element was removed. This commit changes the ngMessage transclude function to create a new child scope instead, which can be destroyed safely when the message element is removed and the message is detached Fixes #14307
1 parent 7489d56 commit 1441560

File tree

2 files changed

+51
-3
lines changed

2 files changed

+51
-3
lines changed

src/ngMessages/messages.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -665,16 +665,17 @@ function ngMessageDirectiveFactory() {
665665
assignRecords(staticExp);
666666
}
667667

668-
var currentElement, messageCtrl;
668+
var currentElement, currentScope, messageCtrl;
669669
ngMessagesCtrl.register(commentNode, messageCtrl = {
670670
test: function(name) {
671671
return contains(records, name);
672672
},
673673
attach: function() {
674674
if (!currentElement) {
675-
$transclude(scope, function(elm) {
675+
$transclude(function(elm, newScope) {
676676
$animate.enter(elm, null, element);
677677
currentElement = elm;
678+
currentScope = newScope;
678679

679680
// Each time we attach this node to a message we get a new id that we can match
680681
// when we are destroying the node later.
@@ -697,6 +698,8 @@ function ngMessageDirectiveFactory() {
697698
var elm = currentElement;
698699
currentElement = null;
699700
$animate.leave(elm);
701+
currentScope && currentScope.$destroy();
702+
currentScope = null;
700703
}
701704
}
702705
});

test/ngMessages/messagesSpec.js

+46-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
describe('ngMessages', function() {
3+
fdescribe('ngMessages', function() {
44
beforeEach(inject.strictDi());
55
beforeEach(module('ngMessages'));
66

@@ -605,6 +605,51 @@ describe('ngMessages', function() {
605605
});
606606
});
607607

608+
609+
it('should clean-up the ngMessage scope when a message is removed',
610+
inject(function($compile, $rootScope) {
611+
function countWatchers() {
612+
var scope = $rootScope;
613+
var watchers = 0;
614+
615+
while (scope) {
616+
watchers += scope.$$watchers.length;
617+
if (scope.$$childHead) {
618+
scope = scope.$$childHead;
619+
} else if (scope.$$nextSibling) {
620+
scope = scope.$$nextSibling;
621+
} else {
622+
scope = null;
623+
}
624+
}
625+
626+
return watchers;
627+
}
628+
629+
var html =
630+
'<div ng-messages="items">' +
631+
'<div ng-message="a">{{forA}}</div>' +
632+
'</div>';
633+
634+
element = $compile(html)($rootScope);
635+
$rootScope.$apply(function() {
636+
$rootScope.forA = 'A';
637+
$rootScope.items = {a: true};
638+
});
639+
640+
expect(element.text()).toBe('A');
641+
var watchers = countWatchers();
642+
643+
$rootScope.$apply('items.a = false');
644+
645+
expect(element.text()).toBe('');
646+
// We don't know exactly how many watchers are on the scope, only that there should be
647+
// one less now
648+
expect(countWatchers()).toBe(watchers-1);
649+
})
650+
);
651+
652+
608653
describe('when including templates', function() {
609654
they('should work with a dynamic collection model which is managed by ngRepeat',
610655
{'<div ng-messages-include="...">': '<div ng-messages="item">' +

0 commit comments

Comments
 (0)