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

Commit 50b118e

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 50b118e

File tree

2 files changed

+131
-47
lines changed

2 files changed

+131
-47
lines changed

‎src/ngMessages/messages.js

+14-20
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

+117-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,96 @@ 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 ng-message="a">A</div>' +
487+
'<div ng-message="b">B</div>' +
488+
'<div ng-message="c">C</div>');
489+
490+
element = $compile(html)($rootScope);
491+
$rootScope.$apply(function() {
492+
$rootScope.data = {
493+
'a': 1,
494+
'b': 2,
495+
'c': 3
496+
};
497+
});
498+
499+
var includeElement = element[0].querySelector('[ng-messages-include], ng-messages-include');
500+
expect(includeElement).toBeFalsy();
501+
502+
var comment = element[0].childNodes[0];
503+
expect(comment.nodeType).toBe(8);
504+
expect(comment.nodeValue).toBe(' ngMessagesInclude: abc.html ');
505+
});
506+
});
507+
418508
they('should load a remote template using $prop',
419509
{'<div ng-messages-include="...">': '<div ng-messages="data">' +
420510
'<div ng-messages-include="abc.html"></div>' +
@@ -437,7 +527,7 @@ describe('ngMessages', function() {
437527
};
438528
});
439529

440-
expect(element.children().length).toBe(1);
530+
expect(messageChildren(element).length).toBe(1);
441531
expect(trim(element.text())).toEqual("A");
442532

443533
$rootScope.$apply(function() {
@@ -446,15 +536,15 @@ describe('ngMessages', function() {
446536
};
447537
});
448538

449-
expect(element.children().length).toBe(1);
539+
expect(messageChildren(element).length).toBe(1);
450540
expect(trim(element.text())).toEqual("C");
451541
});
452542
});
453543

454544
it('should cache the template after download',
455545
inject(function($rootScope, $compile, $templateCache, $httpBackend) {
456546

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

459549
expect($templateCache.get('tpl')).toBeUndefined();
460550

@@ -484,13 +574,13 @@ describe('ngMessages', function() {
484574

485575
$rootScope.$digest();
486576

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

490580
$httpBackend.flush();
491581
$rootScope.$digest();
492582

493-
expect(element.children().length).toBe(1);
583+
expect(messageChildren(element).length).toBe(1);
494584
expect(trim(element.text())).toEqual("You did not enter a value");
495585
}));
496586

@@ -515,7 +605,7 @@ describe('ngMessages', function() {
515605
};
516606
});
517607

518-
expect(element.children().length).toBe(1);
608+
expect(messageChildren(element).length).toBe(1);
519609
expect(trim(element.text())).toEqual("AAA");
520610

521611
$rootScope.$apply(function() {
@@ -525,7 +615,7 @@ describe('ngMessages', function() {
525615
};
526616
});
527617

528-
expect(element.children().length).toBe(1);
618+
expect(messageChildren(element).length).toBe(1);
529619
expect(trim(element.text())).toEqual("B");
530620

531621
$rootScope.$apply(function() {
@@ -534,7 +624,7 @@ describe('ngMessages', function() {
534624
};
535625
});
536626

537-
expect(element.children().length).toBe(1);
627+
expect(messageChildren(element).length).toBe(1);
538628
expect(trim(element.text())).toEqual("C");
539629
}));
540630

@@ -560,7 +650,7 @@ describe('ngMessages', function() {
560650
};
561651
});
562652

563-
expect(element.children().length).toBe(2);
653+
expect(messageChildren(element).length).toBe(2);
564654
expect(s(element.text())).toContain("13");
565655
});
566656
});
@@ -584,14 +674,14 @@ describe('ngMessages', function() {
584674
};
585675
});
586676

587-
expect(element.children().length).toBe(2);
677+
expect(messageChildren(element).length).toBe(2);
588678
expect(s(element.text())).toEqual("XZ");
589679

590680
$rootScope.$apply(function() {
591681
$rootScope.data.y = {};
592682
});
593683

594-
expect(element.children().length).toBe(3);
684+
expect(messageChildren(element).length).toBe(3);
595685
expect(s(element.text())).toEqual("XYZ");
596686
}));
597687

@@ -616,14 +706,14 @@ describe('ngMessages', function() {
616706
};
617707
});
618708

619-
expect(element.children().length).toBe(2);
709+
expect(messageChildren(element).length).toBe(2);
620710
expect(s(element.text())).toEqual("ZZZX");
621711

622712
$rootScope.$apply(function() {
623713
$rootScope.data.y = {};
624714
});
625715

626-
expect(element.children().length).toBe(3);
716+
expect(messageChildren(element).length).toBe(3);
627717
expect(s(element.text())).toEqual("YYYZZZX");
628718
}));
629719
});

0 commit comments

Comments
 (0)