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>
   ```
This commit is contained in:
Georgios Kalpakas
2016-08-08 13:48:11 +03:00
parent cb31067c2a
commit 13c2522baf
2 changed files with 122 additions and 12 deletions
+40 -11
View File
@@ -1902,12 +1902,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
function compileNodes(nodeList, transcludeFn, $rootElement, maxPriority, ignoreDirective,
previousCompileContext) {
var linkFns = [],
// `nodeList` can be either an element's `.childNodes` (live NodeList)
// or a jqLite/jQuery collection or an array
notLiveList = isArray(nodeList) || (nodeList instanceof jqLite),
attrs, directives, nodeLinkFn, childNodes, childLinkFn, linkFnFound, nodeLinkFnFound;
for (var i = 0; i < nodeList.length; i++) {
attrs = new Attributes();
// we must always refer to nodeList[i] since the nodes can be replaced underneath us.
// Workaround for #11781 and #14924
if (msie === 11) {
mergeConsecutiveTextNodes(nodeList, i, notLiveList);
}
// We must always refer to `nodeList[i]` hereafter,
// since the nodes can be replaced underneath us.
directives = collectDirectives(nodeList[i], [], attrs, i === 0 ? maxPriority : undefined,
ignoreDirective);
@@ -1998,6 +2008,32 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
}
function mergeConsecutiveTextNodes(nodeList, idx, notLiveList) {
var node = nodeList[idx];
var parent = node.parentNode;
var sibling;
if (node.nodeType !== NODE_TYPE_TEXT) {
return;
}
while (true) {
sibling = parent ? node.nextSibling : nodeList[idx + 1];
if (!sibling || sibling.nodeType !== NODE_TYPE_TEXT) {
break;
}
node.nodeValue = node.nodeValue + sibling.nodeValue;
if (sibling.parentNode) {
sibling.parentNode.removeChild(sibling);
}
if (notLiveList && sibling === nodeList[idx + 1]) {
nodeList.splice(idx + 1, 1);
}
}
}
function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) {
function boundTranscludeFn(transcludedScope, cloneFn, controllers, futureParentElement, containingScope) {
@@ -2107,13 +2143,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
break;
case NODE_TYPE_TEXT: /* Text Node */
if (msie === 11) {
// Workaround for #11781
while (node.parentNode && node.nextSibling && node.nextSibling.nodeType === NODE_TYPE_TEXT) {
node.nodeValue = node.nodeValue + node.nextSibling.nodeValue;
node.parentNode.removeChild(node.nextSibling);
}
}
addTextInterpolateDirective(directives, node.nodeValue);
break;
case NODE_TYPE_COMMENT: /* Comment */
@@ -2387,9 +2416,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var slots = createMap();
$template = jqLite(jqLiteClone(compileNode)).contents();
if (isObject(directiveValue)) {
if (!isObject(directiveValue)) {
$template = jqLite(jqLiteClone(compileNode)).contents();
} else {
// We have transclusion slots,
// collect them up, compile them and store their transclusion functions
+82 -1
View File
@@ -3260,6 +3260,26 @@ describe('$compile', function() {
}));
it('should not process text nodes merged into their sibling', inject(function($compile, $rootScope) {
var div = document.createElement('div');
div.appendChild(document.createTextNode('1{{ value }}'));
div.appendChild(document.createTextNode('2{{ value }}'));
div.appendChild(document.createTextNode('3{{ value }}'));
element = jqLite(div.childNodes);
var initialWatcherCount = $rootScope.$countWatchers();
$compile(element)($rootScope);
$rootScope.$apply('value = 0');
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
expect(element.text()).toBe('102030');
expect(newWatcherCount).toBe(3);
dealoc(div);
}));
it('should support custom start/end interpolation symbols in template and directive template',
function() {
module(function($interpolateProvider, $compileProvider) {
@@ -8670,10 +8690,38 @@ describe('$compile', function() {
element = $compile('<div transclude><div child></div></div>')($rootScope);
expect(capturedChildCtrl).toBeTruthy();
});
});
// See issue https://github.com/angular/angular.js/issues/14924
it('should not process top-level transcluded text nodes merged into their sibling',
function() {
module(function() {
directive('transclude', valueFn({
template: '<ng-transclude></ng-transclude>',
transclude: true,
scope: {}
}));
});
inject(function($compile) {
element = jqLite('<div transclude></div>');
element[0].appendChild(document.createTextNode('1{{ value }}'));
element[0].appendChild(document.createTextNode('2{{ value }}'));
element[0].appendChild(document.createTextNode('3{{ value }}'));
var initialWatcherCount = $rootScope.$countWatchers();
$compile(element)($rootScope);
$rootScope.$apply('value = 0');
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
expect(element.text()).toBe('102030');
expect(newWatcherCount).toBe(3);
});
}
);
// see issue https://github.com/angular/angular.js/issues/9413
describe('passing a parent bound transclude function to the link ' +
'function returned from `$compile`', function() {
@@ -10136,6 +10184,39 @@ describe('$compile', function() {
expect(element.children().eq(2).text()).toEqual('dorothy');
});
});
// See issue https://github.com/angular/angular.js/issues/14924
it('should not process top-level transcluded text nodes merged into their sibling',
function() {
module(function() {
directive('transclude', valueFn({
template: '<ng-transclude></ng-transclude>',
transclude: {},
scope: {}
}));
});
inject(function($compile) {
element = jqLite('<div transclude></div>');
element[0].appendChild(document.createTextNode('1{{ value }}'));
element[0].appendChild(document.createTextNode('2{{ value }}'));
element[0].appendChild(document.createTextNode('3{{ value }}'));
var initialWatcherCount = $rootScope.$countWatchers();
$compile(element)($rootScope);
$rootScope.$apply('value = 0');
var newWatcherCount = $rootScope.$countWatchers() - initialWatcherCount;
expect(element.text()).toBe('102030');
expect(newWatcherCount).toBe(3);
if (msie === 11) {
expect(element.find('ng-transclude').contents().length).toBe(1);
}
});
}
);
});