From c4b04cec74a7b8a9f707ea532453da214b51e4d3 Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Thu, 29 May 2014 12:51:48 -0700 Subject: [PATCH 01/14] style(ngTouch): make lint happy --- src/ngTouch/directive/ngClick.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ngTouch/directive/ngClick.js b/src/ngTouch/directive/ngClick.js index 79d1f1a7921e..bc0993679ceb 100644 --- a/src/ngTouch/directive/ngClick.js +++ b/src/ngTouch/directive/ngClick.js @@ -95,8 +95,8 @@ ngTouch.directive('ngClick', ['$parse', '$timeout', '$rootElement', // // Why not just put click handlers on the element? // We do that too, just to be sure. If the tap event caused the DOM to change, - // it is possible another element is now in that position. To take account for these possibly distinct elements, - // the handlers are global and care only about coordinates. + // it is possible another element is now in that position. To take account for these possibly + // distinct elements, the handlers are global and care only about coordinates. // Checks if the coordinates are close enough to be within the region. function hit(x1, y1, x2, y2) { From bea77e4b15dc415ac2961bdadbf3f1d40de0b7eb Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 11:51:19 +0100 Subject: [PATCH 02/14] style($compileSpec): fix typos --- test/ng/compileSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b5cb48ce9799..d86e8e1f7e61 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3972,9 +3972,9 @@ describe('$compile', function() { }); - it('should throw on an ng-translude element inside no transclusion directive', function() { + it('should throw on an ng-transclude element inside no transclusion directive', function() { inject(function ($rootScope, $compile) { - // we need to do this because different browsers print empty attributres differently + // we need to do this because different browsers print empty attributes differently try { $compile('
')($rootScope); } catch(e) { From 1fef5fe8230e8dc53f2c9f3f510a35cf18eeab43 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 12:01:44 +0100 Subject: [PATCH 03/14] fix($compile): pass transcludeFn down to nested transclude directives If you have two directives that both expect to receive transcluded content the outer directive works but the inner directive never receives a transclusion function. This only failed if the first transclude directive was not the first directive found in compilation. Handles the regression identified in e994259739821094e77a3d2c1f30c28713b7ab3a Fixes #7240 Closes #7387 --- src/ng/compile.js | 23 ++++++--- test/ng/compileSpec.js | 109 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 8 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index ce2002c80a73..836419333e14 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -936,7 +936,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return linkFnFound ? compositeLinkFn : null; function compositeLinkFn(scope, nodeList, $rootElement, boundTranscludeFn) { - var nodeLinkFn, childLinkFn, node, $node, childScope, childTranscludeFn, i, ii, n; + var nodeLinkFn, childLinkFn, node, $node, childScope, i, ii, n, childBoundTranscludeFn; // copy nodeList so that linking doesn't break due to live list updates. var nodeListLength = nodeList.length, @@ -958,14 +958,19 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } else { childScope = scope; } - childTranscludeFn = nodeLinkFn.transclude; - if (childTranscludeFn || (!boundTranscludeFn && transcludeFn)) { - nodeLinkFn(childLinkFn, childScope, node, $rootElement, - createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn) - ); + + // We need to create a new boundTranscludeFn if + // - a directive on this element wants to transclude + // or + // - there is no boundTranscludeFn already and a transcludeFn was passed in + if ( nodeLinkFn.transcludeOnThisElement || (!boundTranscludeFn && transcludeFn) ) { + childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn); } else { - nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn); + childBoundTranscludeFn = boundTranscludeFn; } + + nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn); + } else if (childLinkFn) { childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn); } @@ -1341,7 +1346,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true; - nodeLinkFn.transclude = hasTranscludeDirective && childTranscludeFn; + nodeLinkFn.transcludeOnThisElement = hasTranscludeDirective; + nodeLinkFn.transclude = childTranscludeFn; + previousCompileContext.hasElementTranscludeDirective = hasElementTranscludeDirective; // might be normal or delayed nodeLinkFn depending on if templateUrl is present diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index d86e8e1f7e61..4d1610bcd714 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4206,6 +4206,115 @@ describe('$compile', function() { }); }); + + + describe('nested transcludes', function() { + + beforeEach(module(function($compileProvider) { + + $compileProvider.directive('noop', valueFn({})); + + $compileProvider.directive('sync', valueFn({ + template: '
', + transclude: true + })); + + $compileProvider.directive('async', valueFn({ + templateUrl: 'async', + transclude: true + })); + + $compileProvider.directive('syncSync', valueFn({ + template: '
', + transclude: true + })); + + $compileProvider.directive('syncAsync', valueFn({ + template: '
', + transclude: true + })); + + $compileProvider.directive('asyncSync', valueFn({ + templateUrl: 'asyncSync', + transclude: true + })); + + $compileProvider.directive('asyncAsync', valueFn({ + templateUrl: 'asyncAsync', + transclude: true + })); + + })); + + beforeEach(inject(function($templateCache) { + $templateCache.put('async', '
'); + $templateCache.put('asyncSync', '
'); + $templateCache.put('asyncAsync', '
'); + })); + + + it('should allow nested transclude directives with sync template containing sync template', inject(function($compile, $rootScope) { + element = $compile('
transcluded content
')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + it('should allow nested transclude directives with sync template containing async template', inject(function($compile, $rootScope) { + element = $compile('
transcluded content
')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + it('should allow nested transclude directives with async template containing sync template', inject(function($compile, $rootScope) { + element = $compile('
transcluded content
')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + it('should allow nested transclude directives with async template containing asynch template', inject(function($compile, $rootScope) { + element = $compile('
transcluded content
')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + }); + + + describe('multiple siblings receiving transclusion', function() { + + it("should only receive transclude from parent", function() { + + module(function($compileProvider) { + + $compileProvider.directive('myExample', valueFn({ + scope: {}, + link: function link(scope, element, attrs) { + var foo = element[0].querySelector('.foo'); + scope.children = angular.element(foo).children().length; + }, + template: '
' + + '
myExample {{children}}!
' + + '
has children
' + + '
' + + '
', + transclude: true + + })); + + }); + + inject(function($compile, $rootScope) { + var element = $compile('
')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('myExample 0!'); + dealoc(element); + + element = $compile('

')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('myExample 1!has children'); + dealoc(element); + }); + }); + }); }); From ad0c5107ba0d7dd90354607458a1fdc32ace6c79 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 12:09:15 +0100 Subject: [PATCH 04/14] refactor($compile): change parameter name The boundTransclusionFn that is passed in is really the one from the parent node. The change to parentBoundTranscludeFn clarifies this compared to the childBoundTranscludeFn. --- src/ng/compile.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 836419333e14..32f88a41bc8a 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -935,7 +935,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // return a linking function if we have found anything, null otherwise return linkFnFound ? compositeLinkFn : null; - function compositeLinkFn(scope, nodeList, $rootElement, boundTranscludeFn) { + function compositeLinkFn(scope, nodeList, $rootElement, parentBoundTranscludeFn) { var nodeLinkFn, childLinkFn, node, $node, childScope, i, ii, n, childBoundTranscludeFn; // copy nodeList so that linking doesn't break due to live list updates. @@ -962,17 +962,17 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // We need to create a new boundTranscludeFn if // - a directive on this element wants to transclude // or - // - there is no boundTranscludeFn already and a transcludeFn was passed in - if ( nodeLinkFn.transcludeOnThisElement || (!boundTranscludeFn && transcludeFn) ) { + // - there is no parentBoundTranscludeFn already and a transcludeFn was passed in + if ( nodeLinkFn.transcludeOnThisElement || (!parentBoundTranscludeFn && transcludeFn) ) { childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn); } else { - childBoundTranscludeFn = boundTranscludeFn; + childBoundTranscludeFn = parentBoundTranscludeFn; } nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn); } else if (childLinkFn) { - childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn); + childLinkFn(scope, node.childNodes, undefined, parentBoundTranscludeFn); } } } From d414b787173643362c0c513a1929d8e715ca340e Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 12:11:28 +0100 Subject: [PATCH 05/14] fix($compile): fix nested isolated transclude directives Closes #1809 Closes #7499 --- src/ng/compile.js | 18 +++++++++--- test/ng/compileSpec.js | 67 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 32f88a41bc8a..6ad1899f5f1d 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -964,7 +964,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // or // - there is no parentBoundTranscludeFn already and a transcludeFn was passed in if ( nodeLinkFn.transcludeOnThisElement || (!parentBoundTranscludeFn && transcludeFn) ) { - childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn); + childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn, parentBoundTranscludeFn); } else { childBoundTranscludeFn = parentBoundTranscludeFn; } @@ -978,8 +978,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } - function createBoundTranscludeFn(scope, transcludeFn) { - return function boundTranscludeFn(transcludedScope, cloneFn, controllers) { + function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) { + + // If there is a previous boundTransclude function and it has a transclusionScope then + // use this instead of the current scope + scope = previousBoundTranscludeFn && previousBoundTranscludeFn.transclusionScope || scope; + + var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) { var scopeCreated = false; if (!transcludedScope) { @@ -994,6 +999,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } return clone; }; + + // Store the transclusionScope for nested transclusions + boundTranscludeFn.transclusionScope = scope; + + return boundTranscludeFn; } /** @@ -1768,7 +1778,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { safeAddClass(jqLite(linkNode), oldClasses); } if (afterTemplateNodeLinkFn.transclude) { - childBoundTranscludeFn = createBoundTranscludeFn(scope, afterTemplateNodeLinkFn.transclude); + childBoundTranscludeFn = createBoundTranscludeFn(scope, afterTemplateNodeLinkFn.transclude, boundTranscludeFn); } else { childBoundTranscludeFn = boundTranscludeFn; } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 4d1610bcd714..b7d2e7aaf932 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4279,6 +4279,73 @@ describe('$compile', function() { }); + describe('nested isolated scope transcludes', function() { + beforeEach(module(function($compileProvider) { + + $compileProvider.directive('trans', valueFn({ + restrict: 'E', + template: '
', + transclude: true + })); + + $compileProvider.directive('transAsync', valueFn({ + restrict: 'E', + templateUrl: 'transAsync', + transclude: true + })); + + $compileProvider.directive('iso', valueFn({ + restrict: 'E', + transclude: true, + template: '', + scope: {} + })); + $compileProvider.directive('isoAsync1', valueFn({ + restrict: 'E', + transclude: true, + template: '', + scope: {} + })); + $compileProvider.directive('isoAsync2', valueFn({ + restrict: 'E', + transclude: true, + templateUrl: 'isoAsync', + scope: {} + })); + })); + + beforeEach(inject(function($templateCache) { + $templateCache.put('transAsync', '
'); + $templateCache.put('isoAsync', ''); + })); + + + it('should pass the outer scope to the transclude on the isolated template sync-sync', inject(function($compile, $rootScope) { + + $rootScope.val = 'transcluded content'; + element = $compile('')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + it('should pass the outer scope to the transclude on the isolated template async-sync', inject(function($compile, $rootScope) { + + $rootScope.val = 'transcluded content'; + element = $compile('')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + it('should pass the outer scope to the transclude on the isolated template async-async', inject(function($compile, $rootScope) { + + $rootScope.val = 'transcluded content'; + element = $compile('')($rootScope); + $rootScope.$digest(); + expect(element.text()).toEqual('transcluded content'); + })); + + }); + describe('multiple siblings receiving transclusion', function() { it("should only receive transclude from parent", function() { From 6e1546226797cf46ab4bcbe76a1fb7f4ccbc7f74 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 11:44:45 +0100 Subject: [PATCH 06/14] refactor($compile): no need to use bind --- src/ng/compile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 6ad1899f5f1d..b7253679a08b 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -995,7 +995,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var clone = transcludeFn(transcludedScope, cloneFn, controllers); if (scopeCreated) { - clone.on('$destroy', bind(transcludedScope, transcludedScope.$destroy)); + clone.on('$destroy', function() { transcludedScope.$destroy(); }); } return clone; }; From 19af0397456eb8fc06dea47145fdee0e38e62f81 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 12:13:27 +0100 Subject: [PATCH 07/14] fix($compile): don't pass transclude to template of non-transclude directive If a directive provides a template but is not explicitly requesting transclusion then the compiler should not pass a transclusion function to the directives within the template. --- src/ng/compile.js | 25 +++++++++++++++++-------- test/ng/compileSpec.js | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index b7253679a08b..7ae720f4a2d4 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -924,7 +924,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { !childNodes.length) ? null : compileNodes(childNodes, - nodeLinkFn ? nodeLinkFn.transclude : transcludeFn); + nodeLinkFn ? ( + (nodeLinkFn.transcludeOnThisElement || !nodeLinkFn.templateOnThisElement) + && nodeLinkFn.transclude) : transcludeFn); linkFns.push(nodeLinkFn, childLinkFn); linkFnFound = linkFnFound || nodeLinkFn || childLinkFn; @@ -959,14 +961,17 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { childScope = scope; } - // We need to create a new boundTranscludeFn if - // - a directive on this element wants to transclude - // or - // - there is no parentBoundTranscludeFn already and a transcludeFn was passed in - if ( nodeLinkFn.transcludeOnThisElement || (!parentBoundTranscludeFn && transcludeFn) ) { - childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude || transcludeFn, parentBoundTranscludeFn); - } else { + if ( nodeLinkFn.transcludeOnThisElement ) { + childBoundTranscludeFn = createBoundTranscludeFn(scope, nodeLinkFn.transclude, parentBoundTranscludeFn); + + } else if (!nodeLinkFn.templateOnThisElement && parentBoundTranscludeFn) { childBoundTranscludeFn = parentBoundTranscludeFn; + + } else if (!parentBoundTranscludeFn && transcludeFn) { + childBoundTranscludeFn = createBoundTranscludeFn(scope, transcludeFn); + + } else { + childBoundTranscludeFn = null; } nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn); @@ -1181,6 +1186,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { templateDirective = previousCompileContext.templateDirective, nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective, hasTranscludeDirective = false, + hasTemplate = false, hasElementTranscludeDirective = previousCompileContext.hasElementTranscludeDirective, $compileNode = templateAttrs.$$element = jqLite(compileNode), directive, @@ -1271,6 +1277,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (directive.template) { + hasTemplate = true; assertNoDuplicate('template', templateDirective, directive, $compileNode); templateDirective = directive; @@ -1320,6 +1327,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (directive.templateUrl) { + hasTemplate = true; assertNoDuplicate('template', templateDirective, directive, $compileNode); templateDirective = directive; @@ -1357,6 +1365,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { nodeLinkFn.scope = newScopeDirective && newScopeDirective.scope === true; nodeLinkFn.transcludeOnThisElement = hasTranscludeDirective; + nodeLinkFn.templateOnThisElement = hasTemplate; nodeLinkFn.transclude = childTranscludeFn; previousCompileContext.hasElementTranscludeDirective = hasElementTranscludeDirective; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b7d2e7aaf932..cce526d17363 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3988,6 +3988,39 @@ describe('$compile', function() { }); + it('should not pass transclusion into a template directive when the directive didn\'t request transclusion', function() { + + module(function($compileProvider) { + + $compileProvider.directive('transFoo', valueFn({ + template: '
' + + '
' + + '
this one should get replaced with content
' + + '
' + + '
', + transclude: true + + })); + + $compileProvider.directive('noTransBar', valueFn({ + template: '
' + + // This ng-transclude is invalid. It should throw an error. + '
' + + '
', + transclude: false + + })); + }); + + inject(function($compile, $rootScope) { + expect(function() { + $compile('
content
')($rootScope); + }).toThrowMinErr('ngTransclude', 'orphan', + 'Illegal use of ngTransclude directive in the template! No parent directive that requires a transclusion found. Element:
'); + }); + }); + + it('should make the result of a transclusion available to the parent directive in post-linking phase' + '(template)', function() { module(function() { From d71df9f83cd3882295ca01b1bb8ad7fb024165b6 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 11:45:03 +0100 Subject: [PATCH 08/14] fix(ngIf): ensure that the correct (transcluded) scope is used --- src/ng/directive/ngIf.js | 4 ++-- test/ng/directive/ngIfSpec.js | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js index a31015b2c492..1bf6106aa632 100644 --- a/src/ng/directive/ngIf.js +++ b/src/ng/directive/ngIf.js @@ -89,8 +89,8 @@ var ngIfDirective = ['$animate', function($animate) { if (toBoolean(value)) { if (!childScope) { - childScope = $scope.$new(); - $transclude(childScope, function (clone) { + $transclude(function (clone, newScope) { + childScope = newScope; clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' '); // Note: We only need the first/last node of the cloned nodes. // However, we need to keep the reference to the jqlite wrapper as it might be changed later diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js index 65c6733ee202..dd24f726dcfe 100755 --- a/test/ng/directive/ngIfSpec.js +++ b/test/ng/directive/ngIfSpec.js @@ -199,6 +199,25 @@ describe('ngIf and transcludes', function() { dealoc(element); }); }); + + + it('should use the correct transcluded scope', function() { + module(function($compileProvider) { + $compileProvider.directive('iso', valueFn({ + restrict: 'E', + transclude: true, + template: '
', + scope: {} + })); + }); + inject(function($compile, $rootScope) { + $rootScope.val = 'transcluded content'; + var element = $compile('')($rootScope); + $rootScope.$digest(); + expect(trim(element.text())).toEqual('transcluded content'); + dealoc(element); + }); + }); }); describe('ngIf animations', function () { From b87e5fc0920915991122ba5dac87b619847b3568 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 11:45:03 +0100 Subject: [PATCH 09/14] fix(ngRepeat): ensure that the correct (transcluded) scope is used --- src/ng/directive/ngRepeat.js | 33 +++++++++++++------------------ test/ng/directive/ngRepeatSpec.js | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 5f977ae694e9..972cd0ea2ced 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -264,7 +264,6 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { // lastBlockMap on the next iteration. nextBlockMap = {}, arrayLength, - childScope, key, value, // key/value of iteration trackById, trackByIdFn, @@ -273,6 +272,17 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { nextBlockOrder = [], elementsToRemove; + var updateScope = function(scope, index) { + scope[valueIdentifier] = value; + if (keyIdentifier) scope[keyIdentifier] = key; + scope.$index = index; + scope.$first = (index === 0); + scope.$last = (index === (arrayLength - 1)); + scope.$middle = !(scope.$first || scope.$last); + // jshint bitwise: false + scope.$odd = !(scope.$even = (index&1) === 0); + // jshint bitwise: true + }; if (isArrayLike(collection)) { collectionKeys = collection; @@ -340,8 +350,6 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { if (block.scope) { // if we have already seen this object, then we need to reuse the // associated scope/element - childScope = block.scope; - nextNode = previousNode; do { nextNode = nextNode.nextSibling; @@ -354,25 +362,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { previousNode = getBlockEnd(block); } else { // new item which we don't know about - childScope = $scope.$new(); - } - - childScope[valueIdentifier] = value; - if (keyIdentifier) childScope[keyIdentifier] = key; - childScope.$index = index; - childScope.$first = (index === 0); - childScope.$last = (index === (arrayLength - 1)); - childScope.$middle = !(childScope.$first || childScope.$last); - // jshint bitwise: false - childScope.$odd = !(childScope.$even = (index&1) === 0); - // jshint bitwise: true - - if (!block.scope) { - $transclude(childScope, function(clone) { + $transclude(function(clone, scope) { + block.scope = scope; clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' '); $animate.enter(clone, null, jqLite(previousNode)); previousNode = clone; - block.scope = childScope; // Note: We only need the first/last node of the cloned nodes. // However, we need to keep the reference to the jqlite wrapper as it might be changed later // by a directive with templateUrl when it's template arrives. @@ -380,6 +374,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { nextBlockMap[block.id] = block; }); } + updateScope(block.scope, index); } lastBlockMap = nextBlockMap; }); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 0cc847e66634..fd8f216a524e 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -1175,6 +1175,26 @@ describe('ngRepeat and transcludes', function() { dealoc(element); }); }); + + + it('should use the correct transcluded scope', function() { + module(function($compileProvider) { + $compileProvider.directive('iso', valueFn({ + restrict: 'E', + transclude: true, + template: '
', + scope: {} + })); + }); + inject(function($compile, $rootScope) { + $rootScope.val = 'transcluded content'; + var element = $compile('')($rootScope); + $rootScope.$digest(); + expect(trim(element.text())).toEqual('transcluded content'); + dealoc(element); + }); + }); + }); describe('ngRepeat animations', function() { From e3003d53421325b9de08e79f45608fdd174cdd83 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 23 May 2014 11:45:03 +0100 Subject: [PATCH 10/14] style(ngRepeat): jshint was complaining about var names --- src/ng/directive/ngRepeat.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 972cd0ea2ced..7298375b27ed 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -291,9 +291,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { trackByIdFn = trackByIdExpFn || trackByIdObjFn; // if object, extract keys, sort them and use to determine order of iteration over obj props collectionKeys = []; - for (key in collection) { - if (collection.hasOwnProperty(key) && key.charAt(0) != '$') { - collectionKeys.push(key); + for (var itemKey in collection) { + if (collection.hasOwnProperty(itemKey) && itemKey.charAt(0) != '$') { + collectionKeys.push(itemKey); } } collectionKeys.sort(); @@ -329,10 +329,10 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { } // remove existing items - for (key in lastBlockMap) { + for (var blockKey in lastBlockMap) { // lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn - if (lastBlockMap.hasOwnProperty(key)) { - block = lastBlockMap[key]; + if (lastBlockMap.hasOwnProperty(blockKey)) { + block = lastBlockMap[blockKey]; elementsToRemove = getBlockElements(block.clone); $animate.leave(elementsToRemove); forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; }); From 2ee29c5da81ffacdc1cabb438f5d125d5e116cb9 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Mon, 26 May 2014 22:21:20 +0100 Subject: [PATCH 11/14] fix($compile): don't pass transcludes to non-transclude templateUrl directives --- src/ng/compile.js | 2 +- test/ng/compileSpec.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 7ae720f4a2d4..139801d860ed 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1336,7 +1336,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } nodeLinkFn = compileTemplateUrl(directives.splice(i, directives.length - i), $compileNode, - templateAttrs, jqCollection, childTranscludeFn, preLinkFns, postLinkFns, { + templateAttrs, jqCollection, hasTranscludeDirective && childTranscludeFn, preLinkFns, postLinkFns, { controllerDirectives: controllerDirectives, newIsolateScopeDirective: newIsolateScopeDirective, templateDirective: templateDirective, diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index cce526d17363..ff44f100227d 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4021,6 +4021,42 @@ describe('$compile', function() { }); + it('should not pass transclusion into a templateUrl directive', function() { + + module(function($compileProvider) { + + $compileProvider.directive('transFoo', valueFn({ + template: '
' + + '
' + + '
this one should get replaced with content
' + + '
' + + '
', + transclude: true + + })); + + $compileProvider.directive('noTransBar', valueFn({ + templateUrl: 'noTransBar.html', + transclude: false + + })); + }); + + inject(function($compile, $rootScope, $templateCache) { + $templateCache.put('noTransBar.html', + '
' + + // This ng-transclude is invalid. It should throw an error. + '
' + + '
'); + + expect(function() { + element = $compile('
content
')($rootScope); + $rootScope.$apply(); + }).toThrowMinErr('ngTransclude', 'orphan', + 'Illegal use of ngTransclude directive in the template! No parent directive that requires a transclusion found. Element:
'); + }); + }); + it('should make the result of a transclusion available to the parent directive in post-linking phase' + '(template)', function() { module(function() { From 0c8a2cd2da3a4a9f5d2ee9c25ea8ed56d74a93ab Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Tue, 27 May 2014 15:02:54 -0700 Subject: [PATCH 12/14] fix($compile): set the iteration state before linking This issue was introduced in b87e5fc0920915991122ba5dac87b619847b3568. The state for each row has to be set up *before* linking. The cloneFn (the function passed into $transclude) is called *before* actual linking and thus it is enough to update the state inside the cloneFn callback. --- src/ng/directive/ngRepeat.js | 3 ++- test/ng/directive/ngRepeatSpec.js | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 7298375b27ed..c89bcdd298fe 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -360,6 +360,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { $animate.move(getBlockElements(block.clone), null, jqLite(previousNode)); } previousNode = getBlockEnd(block); + updateScope(block.scope, index); } else { // new item which we don't know about $transclude(function(clone, scope) { @@ -372,9 +373,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { // by a directive with templateUrl when it's template arrives. block.clone = clone; nextBlockMap[block.id] = block; + updateScope(block.scope, index); }); } - updateScope(block.scope, index); } lastBlockMap = nextBlockMap; }); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index fd8f216a524e..9be6ec667e61 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -1195,6 +1195,21 @@ describe('ngRepeat and transcludes', function() { }); }); + + it('should set the state before linking', function() { + module(function($compileProvider) { + $compileProvider.directive('assertA', valueFn(function(scope) { + // This linking function asserts that a is set. + // If we only test this by asserting binding, it will work even if the value is set later. + expect(scope.a).toBeDefined(); + })); + }); + inject(function($compile, $rootScope) { + var element = $compile('
')($rootScope); + $rootScope.$digest(); + dealoc(element); + }); + }); }); describe('ngRepeat animations', function() { From 56c60218d1e70e3a47e37193a4a48714eeda7d44 Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Tue, 27 May 2014 15:11:45 -0700 Subject: [PATCH 13/14] fix($compile): bound transclusion to correct scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nested isolated transclude directives. This improves/fixes the fix in d414b787173643362c0c513a1929d8e715ca340e. See the changed ng-ifunit test: The template inside ng-if should be bound to the isolate scope of `iso` directive (resp. its child scope). Not to a child of the root scope. This shows the issue with ng-if. It’s however problem with other directives too. Instead of remembering the scope, we pass around the bound parent transclusion. --- src/ng/compile.js | 15 ++++----------- test/ng/directive/ngIfSpec.js | 7 +++++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 139801d860ed..5f8cfcc9766d 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -847,7 +847,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { compileNodes($compileNodes, transcludeFn, $compileNodes, maxPriority, ignoreDirective, previousCompileContext); safeAddClass($compileNodes, 'ng-scope'); - return function publicLinkFn(scope, cloneConnectFn, transcludeControllers){ + return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn){ assertArg(scope, 'scope'); // important!!: we must call our jqLite.clone() since the jQuery one is trying to be smart // and sometimes changes the structure of the DOM. @@ -869,7 +869,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (cloneConnectFn) cloneConnectFn($linkNode, scope); - if (compositeLinkFn) compositeLinkFn(scope, $linkNode, $linkNode); + if (compositeLinkFn) compositeLinkFn(scope, $linkNode, $linkNode, parentBoundTranscludeFn); return $linkNode; }; } @@ -985,10 +985,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) { - // If there is a previous boundTransclude function and it has a transclusionScope then - // use this instead of the current scope - scope = previousBoundTranscludeFn && previousBoundTranscludeFn.transclusionScope || scope; - var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) { var scopeCreated = false; @@ -998,16 +994,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { scopeCreated = true; } - var clone = transcludeFn(transcludedScope, cloneFn, controllers); + var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn); if (scopeCreated) { clone.on('$destroy', function() { transcludedScope.$destroy(); }); } return clone; }; - // Store the transclusionScope for nested transclusions - boundTranscludeFn.transclusionScope = scope; - return boundTranscludeFn; } @@ -1786,7 +1779,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // Copy in CSS classes from original node safeAddClass(jqLite(linkNode), oldClasses); } - if (afterTemplateNodeLinkFn.transclude) { + if (afterTemplateNodeLinkFn.transcludeOnThisElement) { childBoundTranscludeFn = createBoundTranscludeFn(scope, afterTemplateNodeLinkFn.transclude, boundTranscludeFn); } else { childBoundTranscludeFn = boundTranscludeFn; diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js index dd24f726dcfe..b18a4301570d 100755 --- a/test/ng/directive/ngIfSpec.js +++ b/test/ng/directive/ngIfSpec.js @@ -204,9 +204,12 @@ describe('ngIf and transcludes', function() { it('should use the correct transcluded scope', function() { module(function($compileProvider) { $compileProvider.directive('iso', valueFn({ + link: function(scope) { + scope.val = 'value in iso scope'; + }, restrict: 'E', transclude: true, - template: '
', + template: '
val={{val}}-
', scope: {} })); }); @@ -214,7 +217,7 @@ describe('ngIf and transcludes', function() { $rootScope.val = 'transcluded content'; var element = $compile('')($rootScope); $rootScope.$digest(); - expect(trim(element.text())).toEqual('transcluded content'); + expect(trim(element.text())).toEqual('val=value in iso scope-transcluded content'); dealoc(element); }); }); From 440be33d48e776beed9ec76c6d400450a61ceb1e Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Thu, 29 May 2014 11:40:12 -0700 Subject: [PATCH 14/14] test($compile): transcludeFn is available in compile of templateUrl directive --- test/ng/compileSpec.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index ff44f100227d..1095a76b6805 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4057,6 +4057,35 @@ describe('$compile', function() { }); }); + + it('should expose transcludeFn in compile fn even for templateUrl', function() { + module(function() { + directive('transInCompile', valueFn({ + transclude: true, + // template: '
whatever
', + templateUrl: 'foo.html', + compile: function(_, __, transclude) { + return function(scope, element) { + transclude(scope, function(clone, scope) { + element.html(''); + element.append(clone); + }); + }; + } + })); + }); + + inject(function($compile, $rootScope, $templateCache) { + $templateCache.put('foo.html', '
whatever
'); + + compile('
transcluded content
'); + $rootScope.$apply(); + + expect(trim(element.text())).toBe('transcluded content'); + }); + }); + + it('should make the result of a transclusion available to the parent directive in post-linking phase' + '(template)', function() { module(function() {