Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a2f1c58

Browse files
committedMar 23, 2016
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 e34ef23 commit a2f1c58

File tree

2 files changed

+51
-3
lines changed

2 files changed

+51
-3
lines changed
 

‎src/ngMessages/messages.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -654,16 +654,17 @@ function ngMessageDirectiveFactory() {
654654
assignRecords(staticExp);
655655
}
656656

657-
var currentElement, messageCtrl;
657+
var currentElement, currentScope, messageCtrl;
658658
ngMessagesCtrl.register(commentNode, messageCtrl = {
659659
test: function(name) {
660660
return contains(records, name);
661661
},
662662
attach: function() {
663663
if (!currentElement) {
664-
$transclude(scope, function(elm) {
664+
$transclude(function(elm, newScope) {
665665
$animate.enter(elm, null, element);
666666
currentElement = elm;
667+
currentScope = newScope;
667668

668669
// Each time we attach this node to a message we get a new id that we can match
669670
// when we are destroying the node later.
@@ -686,6 +687,8 @@ function ngMessageDirectiveFactory() {
686687
var elm = currentElement;
687688
currentElement = null;
688689
$animate.leave(elm);
690+
currentScope && currentScope.$destroy();
691+
currentScope = null;
689692
}
690693
}
691694
});

‎test/ngMessages/messagesSpec.js

Lines changed: 46 additions & 1 deletion
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

@@ -485,6 +485,51 @@ describe('ngMessages', function() {
485485
});
486486
});
487487

488+
489+
it('should clean-up the ngMessage scope when a message is removed',
490+
inject(function($compile, $rootScope) {
491+
function countWatchers() {
492+
var scope = $rootScope;
493+
var watchers = 0;
494+
495+
while (scope) {
496+
watchers += scope.$$watchers.length;
497+
if (scope.$$childHead) {
498+
scope = scope.$$childHead;
499+
} else if (scope.$$nextSibling) {
500+
scope = scope.$$nextSibling;
501+
} else {
502+
scope = null;
503+
}
504+
}
505+
506+
return watchers;
507+
}
508+
509+
var html =
510+
'<div ng-messages="items">' +
511+
'<div ng-message="a">{{forA}}</div>' +
512+
'</div>';
513+
514+
element = $compile(html)($rootScope);
515+
$rootScope.$apply(function() {
516+
$rootScope.forA = 'A';
517+
$rootScope.items = {a: true};
518+
});
519+
520+
expect(element.text()).toBe('A');
521+
var watchers = countWatchers();
522+
523+
$rootScope.$apply('items.a = false');
524+
525+
expect(element.text()).toBe('');
526+
// We don't know exactly how many watchers are on the scope, only that there should be
527+
// one less now
528+
expect(countWatchers()).toBe(watchers-1);
529+
})
530+
);
531+
532+
488533
describe('when including templates', function() {
489534
they('should work with a dynamic collection model which is managed by ngRepeat',
490535
{'<div ng-messages-include="...">': '<div ng-messages="item">' +

0 commit comments

Comments
 (0)
This repository has been archived.