From d599fd3434f7ab5889be5a4d913769b185f918eb Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Sun, 14 Jun 2015 19:49:47 +0200 Subject: [PATCH] fix(Compiler): fix text nodes after content tags fixes #2095 --- .../src/render/dom/shadow_dom/content_tag.ts | 23 ++++++++----- .../dom/shadow_dom/shadow_dom_compile_step.ts | 6 ++-- .../render/dom/shadow_dom/content_tag_spec.ts | 34 ++++++++----------- .../shadow_dom_emulation_integration_spec.ts | 26 +++++++++++++- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/modules/angular2/src/render/dom/shadow_dom/content_tag.ts b/modules/angular2/src/render/dom/shadow_dom/content_tag.ts index df2f616d54..bf6f8daca8 100644 --- 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,7 +20,6 @@ class RenderedContent extends ContentStrategy { constructor(contentEl) { super(); this.beginScript = contentEl; - this.endScript = DOM.nextSibling(this.beginScript); this.nodes = []; } @@ -28,15 +27,23 @@ class RenderedContent extends ContentStrategy { // Previous content is removed. insert(nodes: List) { 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 index ed6a5557ba..171dfa3172 100644 --- 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 index a8b332f275..d96eca0c83 100644 --- 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,7 +13,6 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; import {Content} from 'angular2/src/render/dom/shadow_dom/content_tag'; var _scriptStart = ``; -var _scriptEnd = ``; export function main() { describe('Content', function() { @@ -21,35 +20,32 @@ export function main() { var content; beforeEach(() => { - parent = el(`
${_scriptStart}${_scriptEnd}`); - content = DOM.firstChild(parent); + parent = el(`
${_scriptStart}
`); + 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(""), el("")]) + content.init(null); + content.insert([el("A"), el("B")]); - expect(DOM.getInnerHTML(parent)) - .toEqual(`${_scriptStart}${_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("")]); - c.insert([el("")]); + content.init(null); + content.insert([el("A")]); + content.insert([el("B")]); - expect(DOM.getInnerHTML(parent)).toEqual(`${_scriptStart}${_scriptEnd}`); + expect(parent).toHaveText('B'); }); - it("should insert empty list", () => { - var c = new Content(content, ''); - c.init(null); - c.insert([el("")]); - c.insert([]); + it("should clear nodes on inserting an empty list", () => { + content.init(null); + content.insert([el("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 index dee9367d0e..f2359bdede 100644 --- 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: '

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) => {