fix(Compiler): fix text nodes after content tags

vicb · vicb · commit d599fd3434f7 · 2015-06-16T17:38:42.000+02:00
fixes #2095
diff --git a/modules/angular2/src/render/dom/shadow_dom/content_tag.ts b/modules/angular2/src/render/dom/shadow_dom/content_tag.ts
@@ -1,6 +1,6 @@
 import * as ldModule from './light_dom';
 import {DOM} from 'angular2/src/dom/dom_adapter';
-import {isPresent} from 'angular2/src/facade/lang';
+import {isPresent, isBlank} from 'angular2/src/facade/lang';
 import {List, ListWrapper} from 'angular2/src/facade/collection';
 
 class ContentStrategy {
@@ -20,23 +20,30 @@ class RenderedContent extends ContentStrategy {
   constructor(contentEl) {
     super();
     this.beginScript = contentEl;
-    this.endScript = DOM.nextSibling(this.beginScript);
     this.nodes = [];
   }
 
   // Inserts the nodes in between the start and end scripts.
   // Previous content is removed.
   insert(nodes: List</*node*/ any>) {
     this.nodes = nodes;
+
+    if (isBlank(this.endScript)) {
+      // On first invocation, we need to create the end marker
+      this.endScript = DOM.createScriptTag('type', 'ng/contentEnd');
+      DOM.insertAfter(this.beginScript, this.endScript);
+    } else {
+      // On subsequent invocations, only remove all the nodes between the start end end markers
+      this._removeNodes();
+    }
+
     DOM.insertAllBefore(this.endScript, nodes);
-    this._removeNodesUntil(ListWrapper.isEmpty(nodes) ? this.endScript : nodes[0]);
   }
 
-  _removeNodesUntil(node) {
-    var p = DOM.parentElement(this.beginScript);
-    for (var next = DOM.nextSibling(this.beginScript); next !== node;
-         next = DOM.nextSibling(this.beginScript)) {
-      DOM.removeChild(p, next);
+  _removeNodes() {
+    for (var node = DOM.nextSibling(this.beginScript); node !== this.endScript;
+         node = DOM.nextSibling(this.beginScript)) {
+      DOM.remove(node);
     }
   }
 }
diff --git a/modules/angular2/src/render/dom/shadow_dom/shadow_dom_compile_step.ts b/modules/angular2/src/render/dom/shadow_dom/shadow_dom_compile_step.ts
@@ -47,13 +47,15 @@ export class ShadowDomCompileStep implements CompileStep {
     var selector = MapWrapper.get(attrs, 'select');
     selector = isPresent(selector) ? selector : '';
 
+    // The content tag should be replaced by a pair of marker tags (start & end).
+    // The end marker creation is delayed to keep the number of elements constant.
+    // Creating the end marker here would invalidate the parent's textNodeIndices for the subsequent
+    // text nodes
     var contentStart = DOM.createScriptTag('type', 'ng/contentStart');
     if (assertionsEnabled()) {
       DOM.setAttribute(contentStart, 'select', selector);
     }
-    var contentEnd = DOM.createScriptTag('type', 'ng/contentEnd');
     DOM.insertBefore(current.element, contentStart);
-    DOM.insertBefore(current.element, contentEnd);
     DOM.remove(current.element);
 
     current.element = contentStart;
diff --git a/modules/angular2/test/render/dom/shadow_dom/content_tag_spec.ts b/modules/angular2/test/render/dom/shadow_dom/content_tag_spec.ts
@@ -13,43 +13,39 @@ import {DOM} from 'angular2/src/dom/dom_adapter';
 import {Content} from 'angular2/src/render/dom/shadow_dom/content_tag';
 
 var _scriptStart = `<script start=""></script>`;
-var _scriptEnd = `<script end=""></script>`;
 
 export function main() {
   describe('Content', function() {
     var parent;
     var content;
 
     beforeEach(() => {
-      parent = el(`<div>${_scriptStart}${_scriptEnd}`);
-      content = DOM.firstChild(parent);
+      parent = el(`<div>${_scriptStart}</div>`);
+      let contentStartMarker = DOM.firstChild(parent);
+      content = new Content(contentStartMarker, '');
     });
 
     it("should insert the nodes", () => {
-      var c = new Content(content, '');
-      c.init(null);
-      c.insert([el("<a></a>"), el("<b></b>")])
+      content.init(null);
+      content.insert([el("<a>A</a>"), el("<b>B</b>")]);
 
-          expect(DOM.getInnerHTML(parent))
-              .toEqual(`${_scriptStart}<a></a><b></b>${_scriptEnd}`);
+      expect(parent).toHaveText('AB');
     });
 
     it("should remove the nodes from the previous insertion", () => {
-      var c = new Content(content, '');
-      c.init(null);
-      c.insert([el("<a></a>")]);
-      c.insert([el("<b></b>")]);
+      content.init(null);
+      content.insert([el("<a>A</a>")]);
+      content.insert([el("<b>B</b>")]);
 
-      expect(DOM.getInnerHTML(parent)).toEqual(`${_scriptStart}<b></b>${_scriptEnd}`);
+      expect(parent).toHaveText('B');
     });
 
-    it("should insert empty list", () => {
-      var c = new Content(content, '');
-      c.init(null);
-      c.insert([el("<a></a>")]);
-      c.insert([]);
+    it("should clear nodes on inserting an empty list", () => {
+      content.init(null);
+      content.insert([el("<a>A</a>")]);
+      content.insert([]);
 
-      expect(DOM.getInnerHTML(parent)).toEqual(`${_scriptStart}${_scriptEnd}`);
+      expect(parent).toHaveText('');
     });
   });
 }
diff --git a/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts b/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts
@@ -10,7 +10,7 @@ import {
   it,
   xit,
   beforeEachBindings,
-  SpyObject,
+  SpyObject
 } from 'angular2/test_lib';
 
 import {bind} from 'angular2/di';
@@ -34,6 +34,10 @@ import {StyleInliner} from 'angular2/src/render/dom/shadow_dom/style_inliner';
 
 import {DomTestbed} from './dom_testbed';
 
+import {Injectable} from 'angular2/di';
+
+import {Component, View} from 'angular2/annotations';
+
 export function main() {
   describe('ShadowDom integration tests', function() {
     var strategies = {
@@ -60,6 +64,26 @@ export function main() {
       beforeEachBindings(() => { return [strategyBinding, DomTestbed]; });
 
       describe(`${name} shadow dom strategy`, () => {
+        // GH-2095 - https://github.com/angular/angular/issues/2095
+        it('should support text nodes after content tags',
+           inject([DomTestbed, AsyncTestCompleter], (tb, async) => {
+             tb.compileAll([
+                 simple,
+                 new ViewDefinition({
+                   componentId: 'simple',
+                   template: '<content></content><p>P,</p>{{a}}',
+                   directives: []
+                 })
+               ])
+                 .then((protoViewDtos) => {
+                   var rootView = tb.createRootView(protoViewDtos[0]);
+                   var cmpView = tb.createComponentView(rootView.viewRef, 0, protoViewDtos[1]);
+
+                   tb.renderer.setText(cmpView.viewRef, 0, 'text');
+                   expect(tb.rootEl).toHaveText('P,text');
+                   async.done();
+                 });
+           }));
 
         it('should support simple components',
            inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
