Skip to content

Commit 436d4d0

Browse files
committed
fix($compile): use the correct namespace for transcluded svg elements
Via transclusion, svg elements can occur outside an `<svg>` container in an Angular template but are put into an `<svg>` container through compilation and linking. E.g. Given that `svg-container` is a transcluding directive with the following template: ``` <svg ng-transclude></svg> ``` The following markup creates a `<circle>` inside of an `<svg>` element during runtime: ``` <svg-container> <circle></circle> </svg-container> ``` However, this produces non working `<circle>` elements, as svg elements need to be created inside of an `<svg>` element. This change detects for most cases the correct namespace of transcluded content and recreates that content in the correct `<svg>` 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 angular#8494
1 parent 70c4a07 commit 436d4d0

File tree

5 files changed

+222
-15
lines changed

5 files changed

+222
-15
lines changed

src/ng/compile.js

+51-15
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,18 @@
185185
* * `$scope` - Current scope associated with the element
186186
* * `$element` - Current element
187187
* * `$attrs` - Current attributes object for the element
188-
* * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope.
189-
* The scope can be overridden by an optional first argument.
190-
* `function([scope], cloneLinkingFn)`.
188+
* * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope:
189+
* `function([scope], cloneLinkingFn, futureParentNode)`.
190+
* * `scope`: optional argument to override the scope.
191+
* * `cloneLinkingFn`: optional argument to create clones of the original translcuded content.
192+
* * `futureParentNode`:
193+
* * defines the parent to which the `cloneLinkingFn` will add the cloned elements.
194+
* * default: `$element.parent()` resp. `$element` for `transclude:'element'` resp. `transclude:true`.
195+
* * only needed for transcludes that are allowed to contain non html elements (e.g. SVG elements)
196+
* and when the `cloneLinkinFn` is passed,
197+
* as those elements need to created and cloned in a special way when they are defined outside their
198+
* usual containers (e.g. like `<svg>`).
199+
* * See also the `directive.templateNamespace` property.
191200
*
192201
*
193202
* #### `require`
@@ -265,6 +274,10 @@
265274
* one. See the {@link guide/directive#creating-custom-directives_creating-directives_template-expanding-directive
266275
* Directives Guide} for an example.
267276
*
277+
* There very few scenarios were element replacement is required for the application function,
278+
* the main one being reusable custom components that are used within SVG contexts
279+
* (because SVG doesn't work with custom elements in the DOM tree).
280+
*
268281
* #### `transclude`
269282
* compile the content of the element and make it available to the directive.
270283
* Typically used with {@link ng.directive:ngTransclude
@@ -359,10 +372,9 @@
359372
* the directives to use the controllers as a communication channel.
360373
*
361374
* * `transcludeFn` - A transclude linking function pre-bound to the correct transclusion scope.
362-
* The scope can be overridden by an optional first argument. This is the same as the `$transclude`
363-
* parameter of directive controllers.
364-
* `function([scope], cloneLinkingFn)`.
365-
*
375+
* This is the same as the `$transclude`
376+
* parameter of directive controllers, see there for details.
377+
* `function([scope], cloneLinkingFn, futureParentNode)`.
366378
*
367379
* #### Pre-linking function
368380
*
@@ -879,8 +891,18 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
879891
compileNodes($compileNodes, transcludeFn, $compileNodes,
880892
maxPriority, ignoreDirective, previousCompileContext);
881893
safeAddClass($compileNodes, 'ng-scope');
882-
return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn){
894+
var namespace = null;
895+
return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentNode){
883896
assertArg(scope, 'scope');
897+
if (!namespace) {
898+
namespace = detectNamespaceForChildElements(futureParentNode);
899+
if (namespace !== 'html') {
900+
$compileNodes = jqLite(
901+
wrapTemplate(namespace, jqLite('<div>').append($compileNodes).html())
902+
);
903+
}
904+
}
905+
884906
// important!!: we must call our jqLite.clone() since the jQuery one is trying to be smart
885907
// and sometimes changes the structure of the DOM.
886908
var $linkNode = cloneConnectFn
@@ -901,6 +923,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
901923
};
902924
}
903925

926+
function detectNamespaceForChildElements(parentElement) {
927+
// TODO: Make this detect MathML as well...
928+
var node = parentElement && parentElement[0];
929+
if (!node) {
930+
return 'html';
931+
} else {
932+
return node.nodeName !== 'foreignObject' && node.toString().match(/SVG/) ? 'svg': 'html';
933+
}
934+
}
935+
904936
function safeAddClass($element, className) {
905937
try {
906938
$element.addClass(className);
@@ -1024,7 +1056,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
10241056

10251057
function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn, elementTransclusion) {
10261058

1027-
var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) {
1059+
var boundTranscludeFn = function(transcludedScope, cloneFn, controllers, futureParentNode) {
10281060
var scopeCreated = false;
10291061

10301062
if (!transcludedScope) {
@@ -1033,7 +1065,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
10331065
scopeCreated = true;
10341066
}
10351067

1036-
var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn);
1068+
var clone = transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentNode);
10371069
if (scopeCreated && !elementTransclusion) {
10381070
clone.on('$destroy', function() { transcludedScope.$destroy(); });
10391071
}
@@ -1645,20 +1677,24 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
16451677
}
16461678

16471679
// This is the function that is injected as `$transclude`.
1648-
function controllersBoundTransclude(scope, cloneAttachFn) {
1680+
// Note: all arguments are optional!
1681+
function controllersBoundTransclude(scope, cloneAttachFn, futureParentNode) {
16491682
var transcludeControllers;
16501683

1651-
// no scope passed
1652-
if (!cloneAttachFn) {
1684+
// No scope passed in:
1685+
if (!isScope(scope)) {
1686+
futureParentNode = cloneAttachFn;
16531687
cloneAttachFn = scope;
16541688
scope = undefined;
16551689
}
16561690

16571691
if (hasElementTranscludeDirective) {
16581692
transcludeControllers = elementControllers;
16591693
}
1660-
1661-
return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers);
1694+
if (!futureParentNode) {
1695+
futureParentNode = hasElementTranscludeDirective ? $element.parent() : $element;
1696+
}
1697+
return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentNode);
16621698
}
16631699
}
16641700
}

test/ng/compileSpec.js

+111
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,32 @@ describe('$compile', function() {
6363
terminal: true
6464
}));
6565

66+
directive('svgContainer', function() {
67+
return {
68+
template: '<svg width="400" height="400" ng-transclude></svg>',
69+
replace: true,
70+
transclude: true
71+
};
72+
});
73+
74+
directive('svgCircle', function(){
75+
return {
76+
template: '<circle cx="2" cy="2" r="1"></circle>',
77+
templateNamespace: 'svg',
78+
replace: true
79+
};
80+
});
81+
82+
directive('myForeignObject', function(){
83+
return {
84+
template: '<foreignObject width="100" height="100" ng-transclude></foreignObject>',
85+
templateNamespace: 'svg',
86+
replace: true,
87+
transclude: true
88+
};
89+
});
90+
91+
6692
return function(_$compile_, _$rootScope_) {
6793
$rootScope = _$rootScope_;
6894
$compile = _$compile_;
@@ -137,6 +163,91 @@ describe('$compile', function() {
137163
});
138164

139165

166+
describe('svg namespace transcludes', function() {
167+
// this method assumes some sort of sized SVG element is being inspected.
168+
function assertIsValidSvgCircle(elem) {
169+
var unknownElement = Object.prototype.toString.call(elem) === '[object HTMLUnknownElement]';
170+
expect(unknownElement).toBe(false);
171+
var box = elem.getBoundingClientRect();
172+
expect(box.width === 0 && box.height === 0).toBe(false);
173+
}
174+
175+
it('should handle transcluded svg elements', inject(function($compile){
176+
element = jqLite('<div><svg-container>' +
177+
'<circle cx="4" cy="4" r="2"></circle>' +
178+
'</svg-container></div>');
179+
$compile(element.contents())($rootScope);
180+
document.body.appendChild(element[0]);
181+
182+
var circle = element.find('circle');
183+
184+
assertIsValidSvgCircle(circle[0]);
185+
}));
186+
187+
it('should handle custom svg elements inside svg tag', function(){
188+
element = jqLite('<div><svg width="300" height="300">' +
189+
'<svg-circle></svg-circle>' +
190+
'</svg></div>');
191+
$compile(element.contents())($rootScope);
192+
document.body.appendChild(element[0]);
193+
194+
var circle = element.find('circle');
195+
assertIsValidSvgCircle(circle[0]);
196+
});
197+
198+
it('should handle transcluded custom svg elements', inject(function(){
199+
element = jqLite('<div><svg-container>' +
200+
'<svg-circle></svg-circle>' +
201+
'</svg-container></div>');
202+
$compile(element.contents())($rootScope);
203+
document.body.appendChild(element[0]);
204+
205+
var circle = element.find('circle');
206+
assertIsValidSvgCircle(circle[0]);
207+
}));
208+
209+
it('should handle foreignObject', inject(function(){
210+
element = jqLite('<div><svg-container>' +
211+
'<foreignObject width="100" height="100"><div class="test" style="width:20px;height:20px">test</div></foreignObject>' +
212+
'</svg-container></div>');
213+
$compile(element.contents())($rootScope);
214+
document.body.appendChild(element[0]);
215+
216+
var testElem = element.find('div');
217+
expect(testElem[0].toString()).toBe('[object HTMLDivElement]');
218+
var bounds = testElem[0].getBoundingClientRect();
219+
expect(bounds.width === 20 && bounds.height === 20).toBe(true);
220+
}));
221+
222+
it('should handle custom svg containers that transclude to foreignObject that transclude html', inject(function(){
223+
element = jqLite('<div><svg-container>' +
224+
'<my-foreign-object><div class="test" style="width:20px;height:20px">test</div></my-foreign-object>' +
225+
'</svg-container></div>');
226+
$compile(element.contents())($rootScope);
227+
document.body.appendChild(element[0]);
228+
229+
var testElem = element.find('div');
230+
expect(testElem[0].toString()).toBe('[object HTMLDivElement]');
231+
var bounds = testElem[0].getBoundingClientRect();
232+
expect(bounds.width === 20 && bounds.height === 20).toBe(true);
233+
}));
234+
235+
// NOTE: This test may be redundant.
236+
it('should handle custom svg containers that transclude to foreignObject'+
237+
' that transclude to custom svg containers that transclude to custom elements', inject(function(){
238+
element = jqLite('<div><svg-container>' +
239+
'<my-foreign-object><svg-container><svg-circle></svg-circle></svg-container></my-foreign-object>' +
240+
'</svg-container></div>');
241+
$compile(element.contents())($rootScope);
242+
document.body.appendChild(element[0]);
243+
244+
var circle = element.find('circle');
245+
assertIsValidSvgCircle(circle[0]);
246+
}));
247+
248+
});
249+
250+
140251
describe('compile phase', function() {
141252

142253
it('should attach scope to the document node when it is compiled explicitly', inject(function($document){

test/ng/directive/ngIfSpec.js

+19
Original file line numberDiff line numberDiff line change
@@ -351,4 +351,23 @@ describe('ngIf animations', function () {
351351
});
352352
});
353353

354+
it('should work with svg elements when the svg container is transcluded', function() {
355+
module(function($compileProvider) {
356+
$compileProvider.directive('svgContainer', function() {
357+
return {
358+
template: '<svg ng-transclude></svg>',
359+
replace: true,
360+
transclude: true
361+
};
362+
});
363+
});
364+
inject(function($compile, $rootScope) {
365+
element = $compile('<svg-container><circle ng-if="flag"></circle></svg-container>')($rootScope);
366+
$rootScope.flag = true;
367+
$rootScope.$apply();
368+
369+
var circle = element.find('circle');
370+
expect(circle[0].toString()).toMatch(/SVG/);
371+
});
372+
});
354373
});

test/ng/directive/ngRepeatSpec.js

+20
Original file line numberDiff line numberDiff line change
@@ -1387,4 +1387,24 @@ describe('ngRepeat animations', function() {
13871387
})
13881388
);
13891389

1390+
it('should work with svg elements when the svg container is transcluded', function() {
1391+
module(function($compileProvider) {
1392+
$compileProvider.directive('svgContainer', function() {
1393+
return {
1394+
template: '<svg ng-transclude></svg>',
1395+
replace: true,
1396+
transclude: true
1397+
};
1398+
});
1399+
});
1400+
inject(function($compile, $rootScope) {
1401+
element = $compile('<svg-container><circle ng-repeat="r in rows"></circle></svg-container>')($rootScope);
1402+
$rootScope.rows = [1];
1403+
$rootScope.$apply();
1404+
1405+
var circle = element.find('circle');
1406+
expect(circle[0].toString()).toMatch(/SVG/);
1407+
});
1408+
});
1409+
13901410
});

test/ng/directive/ngSwitchSpec.js

+21
Original file line numberDiff line numberDiff line change
@@ -433,4 +433,25 @@ describe('ngSwitch animations', function() {
433433
expect(destroyed).toBe(true);
434434
});
435435
});
436+
437+
it('should work with svg elements when the svg container is transcluded', function() {
438+
module(function($compileProvider) {
439+
$compileProvider.directive('svgContainer', function() {
440+
return {
441+
template: '<svg ng-transclude></svg>',
442+
replace: true,
443+
transclude: true
444+
};
445+
});
446+
});
447+
inject(function($compile, $rootScope) {
448+
element = $compile('<svg-container ng-switch="inc"><circle ng-switch-when="one"></circle>' +
449+
'</svg-container>')($rootScope);
450+
$rootScope.inc = 'one';
451+
$rootScope.$apply();
452+
453+
var circle = element.find('circle');
454+
expect(circle[0].toString()).toMatch(/SVG/);
455+
});
456+
});
436457
});

0 commit comments

Comments
 (0)