diff --git a/src/ng/compile.js b/src/ng/compile.js index f75cb7393f11..a6aeb7420599 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -294,13 +294,20 @@ * compile the content of the element and make it available to the directive. * Typically used with {@link ng.directive:ngTransclude * ngTransclude}. The advantage of transclusion is that the linking function receives a - * transclusion function which is pre-bound to the correct scope. In a typical setup the widget - * creates an `isolate` scope, but the transclusion is not a child, but a sibling of the `isolate` - * scope. This makes it possible for the widget to have private state, and the transclusion to - * be bound to the parent (pre-`isolate`) scope. + * transclusion function which is pre-bound to the scope of the position in the DOM from where + * it was taken. * - * * `true` - transclude the content of the directive. - * * `'element'` - transclude the whole element including any directives defined at lower priority. + * In a typical setup the widget creates an `isolate` scope, but the transcluded + * content has its own **transclusion scope**. While the **transclusion scope** is owned as a child, + * by the **isolate scope**, it prototypically inherits from the original scope from where the + * transcluded content was taken. + * + * This makes it possible for the widget to have private state, and the transclusion to + * be bound to the original (pre-`isolate`) scope. + * + * * `true` - transclude the content (i.e. the child nodes) of the directive's element. + * * `'element'` - transclude the whole of the directive's element including any directives on this + * element that defined at a lower priority than this directive. * *
* **Note:** When testing an element transclude directive you must not place the directive at the root of the @@ -1166,20 +1173,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn, elementTransclusion) { - var boundTranscludeFn = function(transcludedScope, cloneFn, controllers, futureParentElement) { - var scopeCreated = false; + var boundTranscludeFn = function(transcludedScope, cloneFn, controllers, futureParentElement, containingScope) { if (!transcludedScope) { - transcludedScope = scope.$new(); + transcludedScope = scope.$new(false, containingScope); transcludedScope.$$transcluded = true; - scopeCreated = true; } - var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement); - if (scopeCreated && !elementTransclusion) { - clone.on('$destroy', function() { transcludedScope.$destroy(); }); - } - return clone; + return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement); }; return boundTranscludeFn; @@ -1810,7 +1811,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (!futureParentElement) { futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element; } - return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement); + return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild); } } } diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 34e69b1bd325..15eabf7a8896 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -184,12 +184,20 @@ function $RootScopeProvider(){ * When creating widgets, it is useful for the widget to not accidentally read parent * state. * + * @param {Scope} [parent=this] The {@link ng.$rootScope.Scope `Scope`} that will be the `$parent` + * of the newly created scope. Defaults to `this` scope if not provided. + * This is used when creating a transclude scope to correctly place it + * in the scope hierarchy while maintaining the correct prototypical + * inheritance. + * * @returns {Object} The newly created child scope. * */ - $new: function(isolate) { + $new: function(isolate, parent) { var child; + parent = parent || this; + if (isolate) { child = new Scope(); child.$root = this.$root; @@ -213,13 +221,13 @@ function $RootScopeProvider(){ child = new this.$$ChildScope(); } child['this'] = child; - child.$parent = this; - child.$$prevSibling = this.$$childTail; - if (this.$$childHead) { - this.$$childTail.$$nextSibling = child; - this.$$childTail = child; + child.$parent = parent; + child.$$prevSibling = parent.$$childTail; + if (parent.$$childHead) { + parent.$$childTail.$$nextSibling = child; + parent.$$childTail = child; } else { - this.$$childHead = this.$$childTail = child; + parent.$$childHead = parent.$$childTail = child; } return child; }, diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2a0d84d04150..f52fc1faa6c1 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4331,17 +4331,21 @@ describe('$compile', function() { return { transclude: 'content', replace: true, - scope: true, - template: '' + scope: {}, + link: function(scope) { + scope.x='iso'; + }, + template: '' }; }); }); inject(function(log, $rootScope, $compile) { - element = $compile('
T:{{$parent.$id}}-{{$id}};
') + element = $compile('
T:{{x}}-{{$parent.$id}}-{{$id}};
') ($rootScope); + $rootScope.x = 'root'; $rootScope.$apply(); - expect(element.text()).toEqual('W:1-2;T:1-3;'); - expect(jqLite(element.find('span')[0]).text()).toEqual('T:1-3'); + expect(element.text()).toEqual('W:iso-1-2;T:root-2-3;'); + expect(jqLite(element.find('span')[0]).text()).toEqual('T:root-2-3'); expect(jqLite(element.find('span')[1]).text()).toEqual(';'); }); }); @@ -4551,47 +4555,6 @@ describe('$compile', function() { } - it('should remove transclusion scope, when the DOM is destroyed', function() { - module(function() { - directive('box', valueFn({ - transclude: true, - scope: { name: '=', show: '=' }, - template: '

Hello: {{name}}!

', - link: function(scope, element) { - scope.$watch( - 'show', - function(show) { - if (!show) { - element.find('div').find('div').remove(); - } - } - ); - } - })); - }); - inject(function($compile, $rootScope) { - $rootScope.username = 'Misko'; - $rootScope.select = true; - element = $compile( - '
user: {{username}}
') - ($rootScope); - $rootScope.$apply(); - expect(element.text()).toEqual('Hello: Misko!user: Misko'); - - var widgetScope = $rootScope.$$childHead; - var transcludeScope = widgetScope.$$nextSibling; - expect(widgetScope.name).toEqual('Misko'); - expect(widgetScope.$parent).toEqual($rootScope); - expect(transcludeScope.$parent).toEqual($rootScope); - - $rootScope.select = false; - $rootScope.$apply(); - expect(element.text()).toEqual('Hello: Misko!'); - expect(widgetScope.$$nextSibling).toEqual(null); - }); - }); - - it('should add a $$transcluded property onto the transcluded scope', function() { module(function() { directive('trans', function() { @@ -4964,6 +4927,162 @@ describe('$compile', function() { }); + // see issue https://github.com/angular/angular.js/issues/9095 + describe('removing a transcluded element', function() { + + function countScopes($rootScope) { + return [$rootScope].concat( + getChildScopes($rootScope) + ).length; + + function getChildScopes(scope) { + var children = []; + if (!scope.$$childHead) { return children; } + var childScope = scope.$$childHead; + do { + children.push(childScope); + children = children.concat(getChildScopes(childScope)); + } while ((childScope = childScope.$$nextSibling)); + return children; + } + } + + beforeEach(module(function() { + directive('toggle', function() { + return { + transclude: true, + template: '
' + }; + }); + })); + + + it('should not leak the transclude scope when the transcluded content is an element transclusion directive', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '
{{ msg }}
' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + + it('should not leak the transclude scope when the transcluded content is an multi-element transclusion directive', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '
{{ msg }}
' + + '
{{ msg }}
' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + + it('should not leak the transclude scope if the transcluded contains only comments', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some comment'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some comment'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some comment'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some comment'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + it('should not leak the transclude scope if the transcluded contains only text nodes', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + 'some text' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some text'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some text'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some text'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some text'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + }); + + describe('nested transcludes', function() { beforeEach(module(function($compileProvider) { diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index e3168ec10e86..1331a97f7d20 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -72,6 +72,15 @@ describe('Scope', function() { expect(child.$new).toBe($rootScope.$new); expect(child.$root).toBe($rootScope); })); + + it("should attach the child scope to a specified parent", inject(function($rootScope) { + var isolated = $rootScope.$new(true); + var trans = $rootScope.$new(false, isolated); + $rootScope.a = 123; + expect(isolated.a).toBeUndefined(); + expect(trans.a).toEqual(123); + expect(trans.$parent).toBe(isolated); + })); });