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

Commit d7ec5f3

Browse files
matskomatsko
committed
fix(ngMessages): ensure that multi-level transclusion works with ngMessagesInclude
ngRepeat and any other directives that alter the DOM structure using transclusion may cause ngMessagesInclude to behave in an unpredictable manner. This fix ensures that the element containing the ngMessagesInclude directive will stay in the DOM to avoid these issues. Closes #11196
1 parent bfd7b22 commit d7ec5f3

File tree

2 files changed

+123
-47
lines changed

2 files changed

+123
-47
lines changed

‎src/ngMessages/messages.js‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -500,27 +500,21 @@ angular.module('ngMessages', [])
500500

501501
return {
502502
restrict: 'AE',
503-
require: '^^ngMessages',
504-
compile: function(element, attrs) {
505-
var comment = jqLite($document[0].createComment(' ngMessagesInclude: '));
506-
element.after(comment);
507-
508-
return function($scope, $element, attrs, ngMessagesCtrl) {
509-
// we're removing this since we only need access to the newly
510-
// created comment node as an anchor.
511-
element.remove();
512-
513-
$templateRequest(attrs.ngMessagesInclude || attrs.src).then(function(html) {
514-
var elements = jqLite('<div></div>').html(html).contents();
515-
var cursor = comment;
516-
forEach(elements, function(elm) {
517-
elm = jqLite(elm);
518-
cursor.after(elm);
519-
cursor = elm;
520-
});
521-
$compile(elements)($scope);
503+
require: '^^ngMessages', // we only require this for validation sake
504+
link: function($scope, element, attrs) {
505+
var src = attrs.ngMessagesInclude || attrs.src;
506+
$templateRequest(src).then(function(html) {
507+
$compile(html)($scope, function(contents) {
508+
element.after(contents);
509+
510+
// the anchor is placed for debugging purposes
511+
var anchor = jqLite($document[0].createComment(' ngMessagesInclude: ' + src + ' '));
512+
element.after(anchor);
513+
514+
// we don't want to pollute the DOM anymore by keeping an empty directive element
515+
element.remove();
522516
});
523-
};
517+
});
524518
}
525519
};
526520
}])

‎test/ngMessages/messagesSpec.js‎

Lines changed: 109 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ describe('ngMessages', function() {
44
beforeEach(inject.strictDi());
55
beforeEach(module('ngMessages'));
66

7+
function messageChildren(element) {
8+
return (element.length ? element[0] : element).querySelectorAll('[ng-message], [ng-message-exp]');
9+
}
10+
711
function they(msg, vals, spec, focus) {
812
forEach(vals, function(val, key) {
913
var m = msg.replace('$prop', key);
@@ -236,7 +240,7 @@ describe('ngMessages', function() {
236240
$rootScope.col = {};
237241
});
238242

239-
expect(element.children().length).toBe(0);
243+
expect(messageChildren(element).length).toBe(0);
240244
expect(trim(element.text())).toEqual('');
241245

242246
$rootScope.$apply(function() {
@@ -246,7 +250,7 @@ describe('ngMessages', function() {
246250
};
247251
});
248252

249-
expect(element.children().length).toBe(1);
253+
expect(messageChildren(element).length).toBe(1);
250254
expect(trim(element.text())).toEqual('This message is blue');
251255

252256
$rootScope.$apply(function() {
@@ -255,13 +259,13 @@ describe('ngMessages', function() {
255259
};
256260
});
257261

258-
expect(element.children().length).toBe(1);
262+
expect(messageChildren(element).length).toBe(1);
259263
expect(trim(element.text())).toEqual('This message is red');
260264

261265
$rootScope.$apply(function() {
262266
$rootScope.col = null;
263267
});
264-
expect(element.children().length).toBe(0);
268+
expect(messageChildren(element).length).toBe(0);
265269
expect(trim(element.text())).toEqual('');
266270

267271

@@ -272,7 +276,7 @@ describe('ngMessages', function() {
272276
};
273277
});
274278

275-
expect(element.children().length).toBe(0);
279+
expect(messageChildren(element).length).toBe(0);
276280
expect(trim(element.text())).toEqual('');
277281
});
278282
});
@@ -346,29 +350,29 @@ describe('ngMessages', function() {
346350
];
347351
});
348352

349-
expect(messageChildren().length).toBe(0);
353+
expect(messageChildren(element).length).toBe(0);
350354
expect(trim(element.text())).toEqual("");
351355

352356
$rootScope.$apply(function() {
353357
$rootScope.col = { hair: true };
354358
});
355359

356-
expect(messageChildren().length).toBe(1);
360+
expect(messageChildren(element).length).toBe(1);
357361
expect(trim(element.text())).toEqual("Your hair is too long");
358362

359363
$rootScope.$apply(function() {
360364
$rootScope.col = { age: true, hair: true};
361365
});
362366

363-
expect(messageChildren().length).toBe(1);
367+
expect(messageChildren(element).length).toBe(1);
364368
expect(trim(element.text())).toEqual("Your age is incorrect");
365369

366370
$rootScope.$apply(function() {
367371
// remove the age!
368372
$rootScope.items.shift();
369373
});
370374

371-
expect(messageChildren().length).toBe(1);
375+
expect(messageChildren(element).length).toBe(1);
372376
expect(trim(element.text())).toEqual("Your hair is too long");
373377

374378
$rootScope.$apply(function() {
@@ -377,12 +381,8 @@ describe('ngMessages', function() {
377381
$rootScope.col.primary = true;
378382
});
379383

380-
expect(messageChildren().length).toBe(1);
384+
expect(messageChildren(element).length).toBe(1);
381385
expect(trim(element.text())).toEqual("Enter something");
382-
383-
function messageChildren() {
384-
return element[0].querySelectorAll('[ng-message], [ng-message-exp]');
385-
}
386386
}));
387387

388388

@@ -415,6 +415,88 @@ describe('ngMessages', function() {
415415
});
416416

417417
describe('when including templates', function() {
418+
they('should work with a dynamic collection model which is managed by ngRepeat',
419+
{'<div ng-messages-include="...">': '<div ng-messages="item">' +
420+
'<div ng-messages-include="abc.html"></div>' +
421+
'</div>',
422+
'<ng-messages-include src="...">': '<ng-messages for="item">' +
423+
'<ng-messages-include src="abc.html"></ng-messages-include>' +
424+
'</ng-messages>'},
425+
function(html) {
426+
inject(function($compile, $rootScope, $templateCache) {
427+
$templateCache.put('abc.html', '<div ng-message="a">A</div>' +
428+
'<div ng-message="b">B</div>' +
429+
'<div ng-message="c">C</div>');
430+
431+
html = '<div><div ng-repeat="item in items">' + html + '</div></div>';
432+
$rootScope.items = [{},{},{}];
433+
434+
element = $compile(html)($rootScope);
435+
$rootScope.$apply(function() {
436+
$rootScope.items[0].a = true;
437+
$rootScope.items[1].b = true;
438+
$rootScope.items[2].c = true;
439+
});
440+
441+
var elements = element[0].querySelectorAll('[ng-repeat]');
442+
443+
// all three collections should have atleast one error showing up
444+
expect(messageChildren(element).length).toBe(3);
445+
expect(messageChildren(elements[0]).length).toBe(1);
446+
expect(messageChildren(elements[1]).length).toBe(1);
447+
expect(messageChildren(elements[2]).length).toBe(1);
448+
449+
// this is the standard order of the displayed error messages
450+
expect(element.text().trim()).toBe('ABC');
451+
452+
$rootScope.$apply(function() {
453+
$rootScope.items[0].a = false;
454+
$rootScope.items[0].c = true;
455+
456+
$rootScope.items[1].b = false;
457+
458+
$rootScope.items[2].c = false;
459+
$rootScope.items[2].a = true;
460+
});
461+
462+
// with the 2nd item gone and the values changed
463+
// we should see both 1 and 3 changed
464+
expect(element.text().trim()).toBe('CA');
465+
466+
$rootScope.$apply(function() {
467+
// add the value for the 2nd item back
468+
$rootScope.items[1].b = true;
469+
$rootScope.items.reverse();
470+
});
471+
472+
// when reversed we get back to our original value
473+
expect(element.text().trim()).toBe('ABC');
474+
});
475+
});
476+
477+
they('should remove the $prop element and place a comment anchor node where it used to be',
478+
{'<div ng-messages-include="...">': '<div ng-messages="data">' +
479+
'<div ng-messages-include="abc.html"></div>' +
480+
'</div>',
481+
'<ng-messages-include src="...">': '<ng-messages for="data">' +
482+
'<ng-messages-include src="abc.html"></ng-messages-include>' +
483+
'</ng-messages>'},
484+
function(html) {
485+
inject(function($compile, $rootScope, $templateCache) {
486+
$templateCache.put('abc.html', '<div></div>');
487+
488+
element = $compile(html)($rootScope);
489+
$rootScope.$digest();
490+
491+
var includeElement = element[0].querySelector('[ng-messages-include], ng-messages-include');
492+
expect(includeElement).toBeFalsy();
493+
494+
var comment = element[0].childNodes[0];
495+
expect(comment.nodeType).toBe(8);
496+
expect(comment.nodeValue).toBe(' ngMessagesInclude: abc.html ');
497+
});
498+
});
499+
418500
they('should load a remote template using $prop',
419501
{'<div ng-messages-include="...">': '<div ng-messages="data">' +
420502
'<div ng-messages-include="abc.html"></div>' +
@@ -437,7 +519,7 @@ describe('ngMessages', function() {
437519
};
438520
});
439521

440-
expect(element.children().length).toBe(1);
522+
expect(messageChildren(element).length).toBe(1);
441523
expect(trim(element.text())).toEqual("A");
442524

443525
$rootScope.$apply(function() {
@@ -446,15 +528,15 @@ describe('ngMessages', function() {
446528
};
447529
});
448530

449-
expect(element.children().length).toBe(1);
531+
expect(messageChildren(element).length).toBe(1);
450532
expect(trim(element.text())).toEqual("C");
451533
});
452534
});
453535

454536
it('should cache the template after download',
455537
inject(function($rootScope, $compile, $templateCache, $httpBackend) {
456538

457-
$httpBackend.expect('GET', 'tpl').respond(201, 'abc');
539+
$httpBackend.expect('GET', 'tpl').respond(201, '<div>abc</div>');
458540

459541
expect($templateCache.get('tpl')).toBeUndefined();
460542

@@ -484,13 +566,13 @@ describe('ngMessages', function() {
484566

485567
$rootScope.$digest();
486568

487-
expect(element.children().length).toBe(1);
569+
expect(messageChildren(element).length).toBe(1);
488570
expect(trim(element.text())).toEqual("Your value is that of failure");
489571

490572
$httpBackend.flush();
491573
$rootScope.$digest();
492574

493-
expect(element.children().length).toBe(1);
575+
expect(messageChildren(element).length).toBe(1);
494576
expect(trim(element.text())).toEqual("You did not enter a value");
495577
}));
496578

@@ -515,7 +597,7 @@ describe('ngMessages', function() {
515597
};
516598
});
517599

518-
expect(element.children().length).toBe(1);
600+
expect(messageChildren(element).length).toBe(1);
519601
expect(trim(element.text())).toEqual("AAA");
520602

521603
$rootScope.$apply(function() {
@@ -525,7 +607,7 @@ describe('ngMessages', function() {
525607
};
526608
});
527609

528-
expect(element.children().length).toBe(1);
610+
expect(messageChildren(element).length).toBe(1);
529611
expect(trim(element.text())).toEqual("B");
530612

531613
$rootScope.$apply(function() {
@@ -534,7 +616,7 @@ describe('ngMessages', function() {
534616
};
535617
});
536618

537-
expect(element.children().length).toBe(1);
619+
expect(messageChildren(element).length).toBe(1);
538620
expect(trim(element.text())).toEqual("C");
539621
}));
540622

@@ -560,7 +642,7 @@ describe('ngMessages', function() {
560642
};
561643
});
562644

563-
expect(element.children().length).toBe(2);
645+
expect(messageChildren(element).length).toBe(2);
564646
expect(s(element.text())).toContain("13");
565647
});
566648
});
@@ -584,14 +666,14 @@ describe('ngMessages', function() {
584666
};
585667
});
586668

587-
expect(element.children().length).toBe(2);
669+
expect(messageChildren(element).length).toBe(2);
588670
expect(s(element.text())).toEqual("XZ");
589671

590672
$rootScope.$apply(function() {
591673
$rootScope.data.y = {};
592674
});
593675

594-
expect(element.children().length).toBe(3);
676+
expect(messageChildren(element).length).toBe(3);
595677
expect(s(element.text())).toEqual("XYZ");
596678
}));
597679

@@ -616,14 +698,14 @@ describe('ngMessages', function() {
616698
};
617699
});
618700

619-
expect(element.children().length).toBe(2);
701+
expect(messageChildren(element).length).toBe(2);
620702
expect(s(element.text())).toEqual("ZZZX");
621703

622704
$rootScope.$apply(function() {
623705
$rootScope.data.y = {};
624706
});
625707

626-
expect(element.children().length).toBe(3);
708+
expect(messageChildren(element).length).toBe(3);
627709
expect(s(element.text())).toEqual("YYYZZZX");
628710
}));
629711
});

0 commit comments

Comments
 (0)