From bd48c927d0e696dc6a2e0aee9a6b2fbfe827254f Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 19 Mar 2015 10:04:42 +0100 Subject: [PATCH] fix(ViewContainer) removeChild called with null parent In view_container.js, templateElement.parentNode can be null when two template tags are nested in one another. Accessing the parent node through view.nodes[0].parentNode fixes the problem. closes #997 Closes #999 --- .../src/core/compiler/view_container.js | 9 +++-- modules/angular2/test/directives/for_spec.js | 25 +++++++++++++ modules/angular2/test/directives/if_spec.js | 35 +++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/modules/angular2/src/core/compiler/view_container.js b/modules/angular2/src/core/compiler/view_container.js index cd7f3c226c..1b71ab24bb 100644 --- a/modules/angular2/src/core/compiler/view_container.js +++ b/modules/angular2/src/core/compiler/view_container.js @@ -130,7 +130,7 @@ export class ViewContainer { var detachedView = this.get(atIndex); ListWrapper.removeAt(this._views, atIndex); if (isBlank(this._lightDom)) { - ViewContainer.removeViewNodesFromParent(this.templateElement.parentNode, detachedView); + ViewContainer.removeViewNodes(detachedView); } else { this._lightDom.redistribute(); } @@ -173,8 +173,11 @@ export class ViewContainer { } } - static removeViewNodesFromParent(parent, view) { - for (var i = view.nodes.length - 1; i >= 0; --i) { + static removeViewNodes(view) { + var len = view.nodes.length; + if (len == 0) return; + var parent = view.nodes[0].parentNode; + for (var i = len - 1; i >= 0; --i) { DOM.removeChild(parent, view.nodes[i]); } } diff --git a/modules/angular2/test/directives/for_spec.js b/modules/angular2/test/directives/for_spec.js index 61b06b116d..ffdf338258 100644 --- a/modules/angular2/test/directives/for_spec.js +++ b/modules/angular2/test/directives/for_spec.js @@ -214,6 +214,31 @@ export function main() { cd.detectChanges(); cd.detectChanges(); expect(DOM.getText(view.nodes[0])).toEqual('a-2;b-2;|c-1;|'); + + component.items = [['e'], ['f', 'g']]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('e-1;|f-2;g-2;|'); + + async.done(); + }); + })); + + it('should repeat over nested arrays with no intermediate element', inject([AsyncTestCompleter], (async) => { + compileWithTemplate( + '
' + ).then((pv) => { + createView(pv); + + component.items = [['a', 'b'], ['c']]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('a-2;b-2;c-1;'); + + component.items = [['e'], ['f', 'g']]; + cd.detectChanges(); + expect(DOM.getText(view.nodes[0])).toEqual('e-1;f-2;g-2;'); async.done(); }); })); diff --git a/modules/angular2/test/directives/if_spec.js b/modules/angular2/test/directives/if_spec.js index edbc116a4a..3f0e4479f6 100644 --- a/modules/angular2/test/directives/if_spec.js +++ b/modules/angular2/test/directives/if_spec.js @@ -116,6 +116,39 @@ export function main() { }); })); + it('should handle nested if correctly', inject([AsyncTestCompleter], (async) => { + compileWithTemplate('
').then((pv) => { + createView(pv); + + component.booleanCondition = false; + cd.detectChanges(); + expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0); + expect(DOM.getText(view.nodes[0])).toEqual(''); + + component.booleanCondition = true; + cd.detectChanges(); + expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(1); + expect(DOM.getText(view.nodes[0])).toEqual('hello'); + + component.nestedBooleanCondition = false; + cd.detectChanges(); + expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0); + expect(DOM.getText(view.nodes[0])).toEqual(''); + + component.nestedBooleanCondition = true; + cd.detectChanges(); + expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(1); + expect(DOM.getText(view.nodes[0])).toEqual('hello'); + + component.booleanCondition = false; + cd.detectChanges(); + expect(DOM.querySelectorAll(view.nodes[0], 'copy-me').length).toEqual(0); + expect(DOM.getText(view.nodes[0])).toEqual(''); + + async.done(); + }); + })); + it('should update several nodes with if', inject([AsyncTestCompleter], (async) => { var templateString = '
' + @@ -195,11 +228,13 @@ export function main() { @Component({selector: 'test-cmp'}) class TestComponent { booleanCondition: boolean; + nestedBooleanCondition: boolean; numberCondition: number; stringCondition: string; functionCondition: Function; constructor() { this.booleanCondition = true; + this.nestedBooleanCondition = true; this.numberCondition = 1; this.stringCondition = "foo"; this.functionCondition = function(s, n){