From ffbd276d6def6ff35bfdb30553346e985f4a0de6 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Fri, 22 Aug 2014 11:44:36 -0700 Subject: [PATCH] fix($compile): use the correct namespace for transcluded svg elements Via transclusion, svg elements can occur outside an `` container in an Angular template but are put into an `` container through compilation and linking. E.g. Given that `svg-container` is a transcluding directive with the following template: ``` ``` The following markup creates a `` inside of an `` element during runtime: ``` ``` However, this produces non working `` elements, as svg elements need to be created inside of an `` element. This change detects for most cases the correct namespace of transcluded content and recreates that content in the correct `` container when needed during compilation. For special cases it adds an addition argument to `$transclude` that allows to specify the future parent node of elements that will be cloned and attached using the `cloneAttachFn`. Related to #8494 Closes #8716 --- src/ng/compile.js | 66 ++++++++++---- test/ng/compileSpec.js | 144 ++++++++++++++++++++++++++++++ test/ng/directive/ngIfSpec.js | 19 ++++ test/ng/directive/ngRepeatSpec.js | 20 +++++ test/ng/directive/ngSwitchSpec.js | 21 +++++ 5 files changed, 255 insertions(+), 15 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 3e84c2b0..4cfcf156 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -185,9 +185,18 @@ * * `$scope` - Current scope associated with the element * * `$element` - Current element * * `$attrs` - Current attributes object for the element - * * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope. - * The scope can be overridden by an optional first argument. - * `function([scope], cloneLinkingFn)`. + * * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope: + * `function([scope], cloneLinkingFn, futureParentElement)`. + * * `scope`: optional argument to override the scope. + * * `cloneLinkingFn`: optional argument to create clones of the original translcuded content. + * * `futureParentElement`: + * * defines the parent to which the `cloneLinkingFn` will add the cloned elements. + * * default: `$element.parent()` resp. `$element` for `transclude:'element'` resp. `transclude:true`. + * * only needed for transcludes that are allowed to contain non html elements (e.g. SVG elements) + * and when the `cloneLinkinFn` is passed, + * as those elements need to created and cloned in a special way when they are defined outside their + * usual containers (e.g. like ``). + * * See also the `directive.templateNamespace` property. * * * #### `require` @@ -265,6 +274,10 @@ * one. See the {@link guide/directive#creating-custom-directives_creating-directives_template-expanding-directive * Directives Guide} for an example. * + * There very few scenarios were element replacement is required for the application function, + * the main one being reusable custom components that are used within SVG contexts + * (because SVG doesn't work with custom elements in the DOM tree). + * * #### `transclude` * compile the content of the element and make it available to the directive. * Typically used with {@link ng.directive:ngTransclude @@ -359,10 +372,9 @@ * the directives to use the controllers as a communication channel. * * * `transcludeFn` - A transclude linking function pre-bound to the correct transclusion scope. - * The scope can be overridden by an optional first argument. This is the same as the `$transclude` - * parameter of directive controllers. - * `function([scope], cloneLinkingFn)`. - * + * This is the same as the `$transclude` + * parameter of directive controllers, see there for details. + * `function([scope], cloneLinkingFn, futureParentElement)`. * * #### Pre-linking function * @@ -879,8 +891,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { compileNodes($compileNodes, transcludeFn, $compileNodes, maxPriority, ignoreDirective, previousCompileContext); safeAddClass($compileNodes, 'ng-scope'); - return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn){ + var namespace = null; + return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentElement){ assertArg(scope, 'scope'); + if (!namespace) { + namespace = detectNamespaceForChildElements(futureParentElement); + if (namespace !== 'html') { + $compileNodes = jqLite( + wrapTemplate(namespace, jqLite('
').append($compileNodes).html()) + ); + } + } + // important!!: we must call our jqLite.clone() since the jQuery one is trying to be smart // and sometimes changes the structure of the DOM. var $linkNode = cloneConnectFn @@ -901,6 +923,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { }; } + function detectNamespaceForChildElements(parentElement) { + // TODO: Make this detect MathML as well... + var node = parentElement && parentElement[0]; + if (!node) { + return 'html'; + } else { + return nodeName_(node) !== 'foreignobject' && node.toString().match(/SVG/) ? 'svg': 'html'; + } + } + function safeAddClass($element, className) { try { $element.addClass(className); @@ -1024,7 +1056,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn, elementTransclusion) { - var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) { + var boundTranscludeFn = function(transcludedScope, cloneFn, controllers, futureParentElement) { var scopeCreated = false; if (!transcludedScope) { @@ -1033,7 +1065,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { scopeCreated = true; } - var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn); + var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement); if (scopeCreated && !elementTransclusion) { clone.on('$destroy', function() { transcludedScope.$destroy(); }); } @@ -1645,11 +1677,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } // This is the function that is injected as `$transclude`. - function controllersBoundTransclude(scope, cloneAttachFn) { + // Note: all arguments are optional! + function controllersBoundTransclude(scope, cloneAttachFn, futureParentElement) { var transcludeControllers; - // no scope passed - if (!cloneAttachFn) { + // No scope passed in: + if (!isScope(scope)) { + futureParentElement = cloneAttachFn; cloneAttachFn = scope; scope = undefined; } @@ -1657,8 +1691,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (hasElementTranscludeDirective) { transcludeControllers = elementControllers; } - - return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers); + if (!futureParentElement) { + futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element; + } + return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement); } } } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 7701dd32..10d10623 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -26,6 +26,12 @@ describe('$compile', function() { return !isUnknownElement(d.firstChild); } + // IE9-11 do not support foreignObject in svg... + function supportsForeignObject() { + var d = document.createElementNS('http://www.w3.org/2000/svg', 'foreignObject'); + return !!d.toString().match(/SVGForeignObject/); + } + var element, directive, $compile, $rootScope; beforeEach(module(provideLog, function($provide, $compileProvider){ @@ -80,6 +86,45 @@ describe('$compile', function() { terminal: true })); + directive('svgContainer', function() { + return { + template: '', + replace: true, + transclude: true + }; + }); + + directive('svgCustomTranscludeContainer', function() { + return { + template: '', + transclude: true, + link: function(scope, element, attr, ctrls, $transclude) { + var futureParent = element.children().eq(0); + $transclude(function(clone) { + futureParent.append(clone); + }, futureParent); + } + }; + }); + + directive('svgCircle', function(){ + return { + template: '', + templateNamespace: 'svg', + replace: true + }; + }); + + directive('myForeignObject', function(){ + return { + template: '', + templateNamespace: 'svg', + replace: true, + transclude: true + }; + }); + + return function(_$compile_, _$rootScope_) { $rootScope = _$rootScope_; $compile = _$compile_; @@ -154,6 +199,105 @@ describe('$compile', function() { }); + describe('svg namespace transcludes', function() { + // this method assumes some sort of sized SVG element is being inspected. + function assertIsValidSvgCircle(elem) { + expect(isUnknownElement(elem)).toBe(false); + expect(isSVGElement(elem)).toBe(true); + var box = elem.getBoundingClientRect(); + expect(box.width === 0 && box.height === 0).toBe(false); + } + + it('should handle transcluded svg elements', inject(function($compile){ + element = jqLite('
' + + '' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var circle = element.find('circle'); + + assertIsValidSvgCircle(circle[0]); + })); + + it('should handle custom svg elements inside svg tag', inject(function(){ + element = jqLite('
' + + '' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var circle = element.find('circle'); + assertIsValidSvgCircle(circle[0]); + })); + + it('should handle transcluded custom svg elements', inject(function(){ + element = jqLite('
' + + '' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var circle = element.find('circle'); + assertIsValidSvgCircle(circle[0]); + })); + + if (supportsForeignObject()) { + it('should handle foreignObject', inject(function(){ + element = jqLite('
' + + '
test
' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var testElem = element.find('div'); + expect(isHTMLElement(testElem[0])).toBe(true); + var bounds = testElem[0].getBoundingClientRect(); + expect(bounds.width === 20 && bounds.height === 20).toBe(true); + })); + + it('should handle custom svg containers that transclude to foreignObject that transclude html', inject(function(){ + element = jqLite('
' + + '
test
' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var testElem = element.find('div'); + expect(isHTMLElement(testElem[0])).toBe(true); + var bounds = testElem[0].getBoundingClientRect(); + expect(bounds.width === 20 && bounds.height === 20).toBe(true); + })); + + // NOTE: This test may be redundant. + it('should handle custom svg containers that transclude to foreignObject'+ + ' that transclude to custom svg containers that transclude to custom elements', inject(function(){ + element = jqLite('
' + + '' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var circle = element.find('circle'); + assertIsValidSvgCircle(circle[0]); + })); + } + + it('should handle directives with templates that manually add the transclude further down', inject(function() { + element = jqLite('
' + + '' + + '
'); + $compile(element.contents())($rootScope); + document.body.appendChild(element[0]); + + var circle = element.find('circle'); + assertIsValidSvgCircle(circle[0]); + + })); + + }); + + describe('compile phase', function() { it('should attach scope to the document node when it is compiled explicitly', inject(function($document){ diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js index f952500e..acbbb062 100755 --- a/test/ng/directive/ngIfSpec.js +++ b/test/ng/directive/ngIfSpec.js @@ -351,4 +351,23 @@ describe('ngIf animations', function () { }); }); + it('should work with svg elements when the svg container is transcluded', function() { + module(function($compileProvider) { + $compileProvider.directive('svgContainer', function() { + return { + template: '', + replace: true, + transclude: true + }; + }); + }); + inject(function($compile, $rootScope) { + element = $compile('')($rootScope); + $rootScope.flag = true; + $rootScope.$apply(); + + var circle = element.find('circle'); + expect(circle[0].toString()).toMatch(/SVG/); + }); + }); }); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 9f7aada2..b9a0870f 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -1463,4 +1463,24 @@ describe('ngRepeat animations', function() { }) ); + it('should work with svg elements when the svg container is transcluded', function() { + module(function($compileProvider) { + $compileProvider.directive('svgContainer', function() { + return { + template: '', + replace: true, + transclude: true + }; + }); + }); + inject(function($compile, $rootScope) { + element = $compile('')($rootScope); + $rootScope.rows = [1]; + $rootScope.$apply(); + + var circle = element.find('circle'); + expect(circle[0].toString()).toMatch(/SVG/); + }); + }); + }); diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js index 5e48d68f..07cd625b 100644 --- a/test/ng/directive/ngSwitchSpec.js +++ b/test/ng/directive/ngSwitchSpec.js @@ -433,4 +433,25 @@ describe('ngSwitch animations', function() { expect(destroyed).toBe(true); }); }); + + it('should work with svg elements when the svg container is transcluded', function() { + module(function($compileProvider) { + $compileProvider.directive('svgContainer', function() { + return { + template: '', + replace: true, + transclude: true + }; + }); + }); + inject(function($compile, $rootScope) { + element = $compile('' + + '')($rootScope); + $rootScope.inc = 'one'; + $rootScope.$apply(); + + var circle = element.find('circle'); + expect(circle[0].toString()).toMatch(/SVG/); + }); + }); });