Skip to content

Commit 39e40a9

Browse files
Narretzgkalpak
authored andcommitted
fix(ngMessages): don't crash when transcluded message is removed
Under specific circumstances, ngMessages would go into an infinite loop and crash the browser / page: - At least two ngMessage elements are wrapped inside another element (e.g. ngTransclude) - The first message is currently visible - The first message is removed (e.g. when the whole ngMessages element is removed by an ngIf) When a message is removed, it looks for a previous message - in this specific case it would misidentify the second message for a previous message, which would then cause the first message to be marked as the second message's next message, resulting in an infinite loop, and crash. This fix ensures that only messages with an index lower than the starting message are considered previous elements. Fixes angular#14183
1 parent c9b4251 commit 39e40a9

File tree

2 files changed

+89
-4
lines changed

2 files changed

+89
-4
lines changed

src/ngMessages/messages.js

+5-4
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,13 @@ angular.module('ngMessages', [])
441441
ctrl.reRender();
442442
};
443443

444-
function findPreviousMessage(parent, comment) {
444+
function findPreviousMessage(parent, comment, key) {
445445
var prevNode = comment;
446446
var parentLookup = [];
447447
while (prevNode && prevNode !== parent) {
448448
var prevKey = prevNode.$$ngMessageNode;
449-
if (prevKey && prevKey.length) {
449+
if (prevKey && prevKey.length && prevKey < key) {
450+
// Only add elements whose key is actually lower than the starting element's
450451
return messages[prevKey];
451452
}
452453

@@ -466,7 +467,7 @@ angular.module('ngMessages', [])
466467
if (!ctrl.head) {
467468
ctrl.head = messageNode;
468469
} else {
469-
var match = findPreviousMessage(parent, comment);
470+
var match = findPreviousMessage(parent, comment, key);
470471
if (match) {
471472
messageNode.next = match.next;
472473
match.next = messageNode;
@@ -480,7 +481,7 @@ angular.module('ngMessages', [])
480481
function removeMessageNode(parent, comment, key) {
481482
var messageNode = messages[key];
482483

483-
var match = findPreviousMessage(parent, comment);
484+
var match = findPreviousMessage(parent, comment, key);
484485
if (match) {
485486
match.next = messageNode.next;
486487
} else {

test/ngMessages/messagesSpec.js

+84
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,90 @@ describe('ngMessages', function() {
412412
}));
413413

414414

415+
it('should not crash when the messages are transcluded, the first message is visible, and ngMessages is removed by ngIf', function() {
416+
417+
module(function($compileProvider) {
418+
$compileProvider.directive('messageWrap', function() {
419+
return {
420+
transclude: true,
421+
scope: {
422+
col: '=col'
423+
},
424+
template: '<div ng-messages="col"><ng-transclude></ng-transclude></div>'
425+
};
426+
});
427+
});
428+
429+
inject(function($rootScope, $compile) {
430+
431+
element = $compile('<div><div ng-if="show"><div message-wrap col="col">' +
432+
' <div ng-message="required">Fill in the text field.</div>' +
433+
' <div ng-message="extra">Extra error message.</div>' +
434+
'</div></div></div>')($rootScope);
435+
436+
$rootScope.$apply(function() {
437+
$rootScope.show = true;
438+
$rootScope.col = {
439+
required: true,
440+
extra: true
441+
};
442+
});
443+
444+
expect(messageChildren(element).length).toBe(1);
445+
expect(trim(element.text())).toEqual('Fill in the text field.');
446+
447+
$rootScope.$apply(function() {
448+
$rootScope.col.required = true;
449+
$rootScope.col.extra = false;
450+
});
451+
452+
expect(messageChildren(element).length).toBe(1);
453+
expect(trim(element.text())).toEqual('Fill in the text field.');
454+
455+
$rootScope.$apply('show = false');
456+
457+
expect(messageChildren(element).length).toBe(0);
458+
});
459+
});
460+
461+
462+
it('should not crash when the first of two nested messages is removed', function() {
463+
464+
module(function($compileProvider) {
465+
$compileProvider.directive('removeMe', function() {
466+
return {
467+
link: function(scope, element) {
468+
scope.$watch('hide', function(newVal) {
469+
if (newVal === true) element.remove();
470+
});
471+
}
472+
};
473+
});
474+
});
475+
476+
inject(function($rootScope, $compile) {
477+
478+
element = $compile('<div ng-messages="col"><div class="wrapper">' +
479+
' <div remove-me ng-message="required">Fill in the text field.</div>' +
480+
' <div ng-message="extra">Extra error message.</div>' +
481+
'</div></div>')($rootScope);
482+
483+
$rootScope.$apply(function() {
484+
$rootScope.col = {
485+
required: true,
486+
extra: false
487+
};
488+
});
489+
490+
expect(messageChildren(element).length).toBe(1);
491+
expect(trim(element.text())).toEqual('Fill in the text field.');
492+
493+
$rootScope.$apply('hide = true');
494+
495+
expect(messageChildren(element).length).toBe(0);
496+
});
497+
});
498+
415499
// issue #12856
416500
it('should only detach the message object that is associated with the message node being removed',
417501
inject(function($rootScope, $compile, $animate) {

0 commit comments

Comments
 (0)