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

Commit 3360b44

Browse files
authored
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 PR (#14308)
1 parent e986565 commit 3360b44

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

src/ngMessages/messages.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ function ngMessageDirectiveFactory() {
684684
},
685685
attach: function() {
686686
if (!currentElement) {
687-
$transclude(scope, function(elm) {
687+
$transclude(function(elm, newScope) {
688688
$animate.enter(elm, null, element);
689689
currentElement = elm;
690690

@@ -700,6 +700,7 @@ function ngMessageDirectiveFactory() {
700700
ngMessagesCtrl.deregister(commentNode);
701701
messageCtrl.detach();
702702
}
703+
newScope.$destroy();
703704
});
704705
});
705706
}

test/ngMessages/messagesSpec.js

+28
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,34 @@ describe('ngMessages', function() {
609609
});
610610
});
611611

612+
613+
it('should clean-up the ngMessage scope when a message is removed',
614+
inject(function($compile, $rootScope) {
615+
616+
var html =
617+
'<div ng-messages="items">' +
618+
'<div ng-message="a">{{forA}}</div>' +
619+
'</div>';
620+
621+
element = $compile(html)($rootScope);
622+
$rootScope.$apply(function() {
623+
$rootScope.forA = 'A';
624+
$rootScope.items = {a: true};
625+
});
626+
627+
expect(element.text()).toBe('A');
628+
var watchers = $rootScope.$countWatchers();
629+
630+
$rootScope.$apply('items.a = false');
631+
632+
expect(element.text()).toBe('');
633+
// We don't know exactly how many watchers are on the scope, only that there should be
634+
// one less now
635+
expect($rootScope.$countWatchers()).toBe(watchers - 1);
636+
})
637+
);
638+
639+
612640
describe('when including templates', function() {
613641
they('should work with a dynamic collection model which is managed by ngRepeat',
614642
{'<div ng-messages-include="...">': '<div ng-messages="item">' +

0 commit comments

Comments
 (0)