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

Commit 13c2522

Browse files
committedAug 25, 2016
fix($compile): correctly merge consecutive text nodes on IE11
As explained in #11781 and #14924, IE11 can (under certain circumstances) break up a text node into multiple consecutive ones, breaking interpolated expressions (e.g. `{{'foo'}}` would become `{{` + `'foo'}}`). To work-around this IE11 bug, #11796 introduced extra logic to merge consecutive text nodes (on IE11 only), which relies on the text nodes' having the same `parentNode`. This approach works fine in the common case, where `compileNodes` is called with a live NodeList object, because removing a text node from its parent will automatically update the latter's `.childNodes` NodeList. It falls short though, when calling `compileNodes` with either a jqLite/jQuery collection or an Array. In fails in two ways: 1. If the text nodes do not have a parent at the moment of compiling, there will be no merging. (This happens for example on directives with `transclude: {...}`.) 2. If the text nodes do have a parent, just removing a text node from its parent does **not** remove it from the collection/array, which means that the merged text nodes will still get compiled and linked (and possibly be displayed in the view). E.g. `['{{text1}}', '{{text2}}', '{{text3}}']` will become `['{{text1}}{{text2}}{{text3}}', '{{text2}}', '{{text3}}']`. -- This commit works around the above problems by: 1. Merging consecutive text nodes in the provided list, even if they have no parent. 2. When merging a text node, explicitly remove it from the list (unless it is a live, auto-updating list). This can nonetheless have undesirable (albeit rare) side effects by overzealously merging text nodes that are not meant to be merged (see the "BREAKING CHANGE" section below). Fixes #14924 Closes #15025 BREAKING CHANGE: **Note:** Everything described below affects **IE11 only**. Previously, consecutive text nodes would not get merged if they had no parent. They will now, which might have unexpectd side effects in the following cases: 1. Passing an array or jqLite/jQuery collection of parent-less text nodes to `$compile` directly: ```js // Assuming: var textNodes = [ document.createTextNode('{{'), document.createTextNode('"foo"'), document.createTextNode('}}') ]; var compiledNodes = $compile(textNodes)($rootScope); // Before: console.log(compiledNodes.length); // 3 console.log(compiledNodes.text()); // {{'foo'}} // After: console.log(compiledNodes.length); // 1 console.log(compiledNodes.text()); // foo // To get the old behavior, compile each node separately: var textNodes = [ document.createTextNode('{{'), document.createTextNode('"foo"'), document.createTextNode('}}') ]; var compiledNodes = angular.element(textNodes.map(function (node) { return $compile(node)($rootScope)[0]; })); ``` 2. Using multi-slot transclusion with non-consecutive, default-content text nodes (that form interpolated expressions when merged): ```js // Assuming the following component: .compoent('someThing', { template: '<ng-transclude><!-- Default content goes here --></ng-transclude>' transclude: { ignored: 'veryImportantContent' } }) ``` ```html <!-- And assuming the following view: --> <some-thing> {{ <very-important-content>Nooot</very-important-content> 'foo'}} </some-thing> <!-- Before: --> <some-thing> <ng-transclude> {{ <-- Two separate 'foo'}} <-- text nodes </ng-transclude> </some-thing> <!-- After: --> <some-thing> <ng-transclude> foo <-- The text nodes were merged into `{{'foo'}}`, which was then interpolated </ng-transclude> </some-thing> <!-- To (visually) get the old behavior, wrap top-level text nodes on --> <!-- multi-slot transclusion directives into `<span>` elements; e.g.: --> <some-thing> <span>{{</span> <very-important-content>Nooot</very-important-content> <span>'foo'}}</span> </some-thing> <!-- Result: --> <some-thing> <ng-transclude> <span>{{</span> <-- Two separate <span>'foo'}}</span> <-- nodes </ng-transclude> </some-thing> ```
1 parent cb31067 commit 13c2522

File tree

2 files changed

+122
-12
lines changed

2 files changed

+122
-12
lines changed
 

‎src/ng/compile.js

+40-11
Original file line numberDiff line numberDiff line change
@@ -1902,12 +1902,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19021902
function compileNodes(nodeList, transcludeFn, $rootElement, maxPriority, ignoreDirective,
19031903
previousCompileContext) {
19041904
var linkFns = [],
1905+
// `nodeList` can be either an element's `.childNodes` (live NodeList)
1906+
// or a jqLite/jQuery collection or an array
1907+
notLiveList = isArray(nodeList) || (nodeList instanceof jqLite),
19051908
attrs, directives, nodeLinkFn, childNodes, childLinkFn, linkFnFound, nodeLinkFnFound;
19061909

1910+
19071911
for (var i = 0; i < nodeList.length; i++) {
19081912
attrs = new Attributes();
19091913

1910-
// we must always refer to nodeList[i] since the nodes can be replaced underneath us.
1914+
// Workaround for #11781 and #14924
1915+
if (msie === 11) {
1916+
mergeConsecutiveTextNodes(nodeList, i, notLiveList);
1917+
}
1918+
1919+
// We must always refer to `nodeList[i]` hereafter,
1920+
// since the nodes can be replaced underneath us.
19111921
directives = collectDirectives(nodeList[i], [], attrs, i === 0 ? maxPriority : undefined,
19121922
ignoreDirective);
19131923

@@ -1998,6 +2008,32 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
19982008
}
19992009
}
20002010

2011+
function mergeConsecutiveTextNodes(nodeList, idx, notLiveList) {
2012+
var node = nodeList[idx];
2013+
var parent = node.parentNode;
2014+
var sibling;
2015+
2016+
if (node.nodeType !== NODE_TYPE_TEXT) {
2017+
return;
2018+
}
2019+
2020+
while (true) {
2021+
sibling = parent ? node.nextSibling : nodeList[idx + 1];
2022+
if (!sibling || sibling.nodeType !== NODE_TYPE_TEXT) {
2023+
break;
2024+
}
2025+
2026+
node.nodeValue = node.nodeValue + sibling.nodeValue;
2027+
2028+
if (sibling.parentNode) {
2029+
sibling.parentNode.removeChild(sibling);
2030+
}
2031+
if (notLiveList && sibling === nodeList[idx + 1]) {
2032+
nodeList.splice(idx + 1, 1);
2033+
}
2034+
}
2035+
}
2036+
20012037
function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) {
20022038
function boundTranscludeFn(transcludedScope, cloneFn, controllers, futureParentElement, containingScope) {
20032039

@@ -2107,13 +2143,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
21072143
}
21082144
break;
21092145
case NODE_TYPE_TEXT: /* Text Node */
2110-
if (msie === 11) {
2111-
// Workaround for #11781
2112-
while (node.parentNode && node.nextSibling && node.nextSibling.nodeType === NODE_TYPE_TEXT) {
2113-
node.nodeValue = node.nodeValue + node.nextSibling.nodeValue;
2114-
node.parentNode.removeChild(node.nextSibling);
2115-
}
2116-
}
21172146
addTextInterpolateDirective(directives, node.nodeValue);
21182147
break;
21192148
case NODE_TYPE_COMMENT: /* Comment */
@@ -2387,9 +2416,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
23872416

23882417
var slots = createMap();
23892418

2390-
$template = jqLite(jqLiteClone(compileNode)).contents();
2391-
2392-
if (isObject(directiveValue)) {
2419+
if (!isObject(directiveValue)) {
2420+
$template = jqLite(jqLiteClone(compileNode)).contents();
2421+
} else {
23932422

23942423
// We have transclusion slots,
23952424
// collect them up, compile them and store their transclusion functions

‎test/ng/compileSpec.js

+82-1
Original file line numberDiff line numberDiff line change
@@ -3260,6 +3260,26 @@ describe('$compile', function() {
32603260
}));
32613261

32623262

3263+
it('should not process text nodes merged into their sibling', inject(function($compile, $rootScope) {
3264+
var div = document.createElement('div');
3265+
div.appendChild(document.createTextNode('1{{ value }}'));
3266+
div.appendChild(document.createTextNode('2{{ value }}'));
3267+
div.appendChild(document.createTextNode('3{{ value }}'));
3268+
3269+
element = jqLite(div.childNodes);
3270+
3271+
var initialWatcherCount = $rootScope.$countWatchers();
3272+
$compile(element)($rootScope);
3273+
$rootScope.$apply('value = 0');
3274+
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
3275+
3276+
expect(element.text()).toBe('102030');
3277+
expect(newWatcherCount).toBe(3);
3278+
3279+
dealoc(div);
3280+
}));
3281+
3282+
32633283
it('should support custom start/end interpolation symbols in template and directive template',
32643284
function() {
32653285
module(function($interpolateProvider, $compileProvider) {
@@ -8670,10 +8690,38 @@ describe('$compile', function() {
86708690
element = $compile('<div transclude><div child></div></div>')($rootScope);
86718691
expect(capturedChildCtrl).toBeTruthy();
86728692
});
8673-
86748693
});
86758694

86768695

8696+
// See issue https://github.com/angular/angular.js/issues/14924
8697+
it('should not process top-level transcluded text nodes merged into their sibling',
8698+
function() {
8699+
module(function() {
8700+
directive('transclude', valueFn({
8701+
template: '<ng-transclude></ng-transclude>',
8702+
transclude: true,
8703+
scope: {}
8704+
}));
8705+
});
8706+
8707+
inject(function($compile) {
8708+
element = jqLite('<div transclude></div>');
8709+
element[0].appendChild(document.createTextNode('1{{ value }}'));
8710+
element[0].appendChild(document.createTextNode('2{{ value }}'));
8711+
element[0].appendChild(document.createTextNode('3{{ value }}'));
8712+
8713+
var initialWatcherCount = $rootScope.$countWatchers();
8714+
$compile(element)($rootScope);
8715+
$rootScope.$apply('value = 0');
8716+
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
8717+
8718+
expect(element.text()).toBe('102030');
8719+
expect(newWatcherCount).toBe(3);
8720+
});
8721+
}
8722+
);
8723+
8724+
86778725
// see issue https://github.com/angular/angular.js/issues/9413
86788726
describe('passing a parent bound transclude function to the link ' +
86798727
'function returned from `$compile`', function() {
@@ -10136,6 +10184,39 @@ describe('$compile', function() {
1013610184
expect(element.children().eq(2).text()).toEqual('dorothy');
1013710185
});
1013810186
});
10187+
10188+
10189+
// See issue https://github.com/angular/angular.js/issues/14924
10190+
it('should not process top-level transcluded text nodes merged into their sibling',
10191+
function() {
10192+
module(function() {
10193+
directive('transclude', valueFn({
10194+
template: '<ng-transclude></ng-transclude>',
10195+
transclude: {},
10196+
scope: {}
10197+
}));
10198+
});
10199+
10200+
inject(function($compile) {
10201+
element = jqLite('<div transclude></div>');
10202+
element[0].appendChild(document.createTextNode('1{{ value }}'));
10203+
element[0].appendChild(document.createTextNode('2{{ value }}'));
10204+
element[0].appendChild(document.createTextNode('3{{ value }}'));
10205+
10206+
var initialWatcherCount = $rootScope.$countWatchers();
10207+
$compile(element)($rootScope);
10208+
$rootScope.$apply('value = 0');
10209+
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
10210+
10211+
expect(element.text()).toBe('102030');
10212+
expect(newWatcherCount).toBe(3);
10213+
10214+
if (msie === 11) {
10215+
expect(element.find('ng-transclude').contents().length).toBe(1);
10216+
}
10217+
});
10218+
}
10219+
);
1013910220
});
1014010221

1014110222

0 commit comments

Comments
 (0)
This repository has been archived.