Skip to content

Commit c753b39

Browse files
matskomatsko
committed
fix(ngMessages): ensure that multi-level transclusion works with ngMessageInclude
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 angular#11196
1 parent bfd7b22 commit c753b39

File tree

2 files changed

+86
-47
lines changed

2 files changed

+86
-47
lines changed

‎src/ngMessages/messages.js

+6-20
Original file line numberDiff line numberDiff line change
@@ -500,27 +500,13 @@ 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+
$templateRequest(attrs.ngMessagesInclude || attrs.src).then(function(html) {
506+
$compile(html)($scope, function(contents) {
507+
element.after(contents);
522508
});
523-
};
509+
});
524510
}
525511
};
526512
}])

‎test/ngMessages/messagesSpec.js

+80-27
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,59 @@ 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+
expect(messageChildren(element).length).toBe(3);
444+
expect(messageChildren(elements[0]).length).toBe(1);
445+
expect(messageChildren(elements[1]).length).toBe(1);
446+
expect(messageChildren(elements[2]).length).toBe(1);
447+
448+
expect(element.text().trim()).toBe('ABC');
449+
450+
$rootScope.$apply(function() {
451+
$rootScope.items[0].a = false;
452+
$rootScope.items[0].c = true;
453+
454+
$rootScope.items[1].b = false;
455+
456+
$rootScope.items[2].c = false;
457+
$rootScope.items[2].a = true;
458+
});
459+
460+
expect(element.text().trim()).toBe('CA');
461+
462+
$rootScope.$apply(function() {
463+
$rootScope.items[1].b = true;
464+
$rootScope.items.reverse();
465+
});
466+
467+
expect(element.text().trim()).toBe('ABC');
468+
});
469+
});
470+
418471
they('should load a remote template using $prop',
419472
{'<div ng-messages-include="...">': '<div ng-messages="data">' +
420473
'<div ng-messages-include="abc.html"></div>' +
@@ -437,7 +490,7 @@ describe('ngMessages', function() {
437490
};
438491
});
439492

440-
expect(element.children().length).toBe(1);
493+
expect(messageChildren(element).length).toBe(1);
441494
expect(trim(element.text())).toEqual("A");
442495

443496
$rootScope.$apply(function() {
@@ -446,15 +499,15 @@ describe('ngMessages', function() {
446499
};
447500
});
448501

449-
expect(element.children().length).toBe(1);
502+
expect(messageChildren(element).length).toBe(1);
450503
expect(trim(element.text())).toEqual("C");
451504
});
452505
});
453506

454507
it('should cache the template after download',
455508
inject(function($rootScope, $compile, $templateCache, $httpBackend) {
456509

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

459512
expect($templateCache.get('tpl')).toBeUndefined();
460513

@@ -484,13 +537,13 @@ describe('ngMessages', function() {
484537

485538
$rootScope.$digest();
486539

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

490543
$httpBackend.flush();
491544
$rootScope.$digest();
492545

493-
expect(element.children().length).toBe(1);
546+
expect(messageChildren(element).length).toBe(1);
494547
expect(trim(element.text())).toEqual("You did not enter a value");
495548
}));
496549

@@ -515,7 +568,7 @@ describe('ngMessages', function() {
515568
};
516569
});
517570

518-
expect(element.children().length).toBe(1);
571+
expect(messageChildren(element).length).toBe(1);
519572
expect(trim(element.text())).toEqual("AAA");
520573

521574
$rootScope.$apply(function() {
@@ -525,7 +578,7 @@ describe('ngMessages', function() {
525578
};
526579
});
527580

528-
expect(element.children().length).toBe(1);
581+
expect(messageChildren(element).length).toBe(1);
529582
expect(trim(element.text())).toEqual("B");
530583

531584
$rootScope.$apply(function() {
@@ -534,7 +587,7 @@ describe('ngMessages', function() {
534587
};
535588
});
536589

537-
expect(element.children().length).toBe(1);
590+
expect(messageChildren(element).length).toBe(1);
538591
expect(trim(element.text())).toEqual("C");
539592
}));
540593

@@ -560,7 +613,7 @@ describe('ngMessages', function() {
560613
};
561614
});
562615

563-
expect(element.children().length).toBe(2);
616+
expect(messageChildren(element).length).toBe(2);
564617
expect(s(element.text())).toContain("13");
565618
});
566619
});
@@ -584,14 +637,14 @@ describe('ngMessages', function() {
584637
};
585638
});
586639

587-
expect(element.children().length).toBe(2);
640+
expect(messageChildren(element).length).toBe(2);
588641
expect(s(element.text())).toEqual("XZ");
589642

590643
$rootScope.$apply(function() {
591644
$rootScope.data.y = {};
592645
});
593646

594-
expect(element.children().length).toBe(3);
647+
expect(messageChildren(element).length).toBe(3);
595648
expect(s(element.text())).toEqual("XYZ");
596649
}));
597650

@@ -616,14 +669,14 @@ describe('ngMessages', function() {
616669
};
617670
});
618671

619-
expect(element.children().length).toBe(2);
672+
expect(messageChildren(element).length).toBe(2);
620673
expect(s(element.text())).toEqual("ZZZX");
621674

622675
$rootScope.$apply(function() {
623676
$rootScope.data.y = {};
624677
});
625678

626-
expect(element.children().length).toBe(3);
679+
expect(messageChildren(element).length).toBe(3);
627680
expect(s(element.text())).toEqual("YYYZZZX");
628681
}));
629682
});

0 commit comments

Comments
 (0)