fix($compile): correctly merge consecutive text nodes on IE11

[gkalpak]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix. (Actually, IE11 bug work-around.)

What is the current behavior? (You can also link to an open issue here)
Under certain circumstances, IE11 will break up text nodes, thus breaking interpolation. In some cases Angular fails to fix this (by merging consecutive text nodes) or compiles the merged nodes twice.
See also #14924.

What is the new behavior (if this is a feature change)?
Angular does a better job merging consecutive text nodes and doesn't compile merged nodes twice.

Does this PR introduce a breaking change?
Yes.

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:
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

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:

   // 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):

   // Assuming the following component:
   .compoent('someThing', {
    template: '<ng-transclude><!-- Default content goes here --></ng-transclude>'
    transclude: {
      ignored: 'veryImportantContent'
    }
   })

   <!-- 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>

[lgalfaso]

I am ok-ish with the PR. Anyhow, please move this fix to a new function that is called at the beginning of compileNodes. Is there a chance that nodeList is a line NodeList without a parent?

[gkalpak]

I am ok-ish with the PR.

Yeah, I am not too happy either 😞

Anyhow, please move this fix to a new function that is called at the beginning of compileNodes.

Done. Or did you mean traverse the whole nodeList before entering the loop?

Is there a chance that nodeList is a line NodeList without a parent?

Not afaict. The only kind of live NodeList that Angular itself is passing is some element's childNodes, so by definition, if they lose their parent, they are removed from the live list. I tried other types of live lists (e.g. document.getElementsByClassName) and elements are also removed from them if they are removed from their parents.

[lgalfaso]

Or did you mean traverse the whole nodeList before entering the loop?

Yep.

This a side, LGTM for 1.6 and 1.5 (for the last one, confirm with Pete)

[gkalpak]

Or did you mean traverse the whole nodeList before entering the loop?

Yep.

My only concern with that is that we are going to miss cases where a compile function adds a text node after the compiled element. It's a very rare usecase (I doubt anyone does that), so I am fine moving that before the loop if you think it's better.

Considering that the merging of consecutive text nodes was not backported to 1.5x (because it is kind of a breaking change), I assume this is for 1.6.x only as well. @petebacondarwin, thoughts?

[Narretz]

Let's merge in 1.6. We should get (almost) immediate feedback from the google app, right?

[endamaco]

I am experiencing exactly the same on Angular 1.6.3 .
My case is with these two spans one after the other and having the second one not binded:
<span>&#xe080;</span> <span>{{::model.myVar}}</span>

[Narretz]

@endamaco You should open a new issue with a reproduction.