Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): use the correct namespace for transcluded svg elements #8716

Closed
wants to merge 2 commits into from

Conversation

tbosch
Copy link
Contributor

@tbosch tbosch commented Aug 21, 2014

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 followingg 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
needs 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 #8494

*
* If no `type` is specified, then the type is considered to be html.
* If no `templateNamespace` is specified, then the namespace is considered to be `html`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe templateNamespace is a better name for this, but it's weird --- A) because these values are not actually namespaces, and B) because it's a breaking change for not much of a real reason given A).

If we're going to call it namespace, the values should probably be proper XML namespaces, but we don't do this because people are going to have a much easier time remembering "svg" than "http://www.w3.org/2000/svg"

IMO we should change the symbol name within $compile --- so that the compiler is easier to understand, but the public api doesn't have an unneeded change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your are right, but I chatted with Igor to leave it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have a better name than "namespace"... (type is really too general)


return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers);
if (!futureParentNode) {
futureParentNode = hasElementTranscludeDirective ? $element.parent() : $element;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If futureParentNode is changed to be a node (like its name implies) instead of a jQuery object then .parent() can change to [0].parentNode to avoid the extra jQuery object...

…espace`

Also corrects the tests for MathML that use `directive.templateNamespace`.

BREAKING CHANGE (within 1.3.0-beta): `directive.type` was renamed to `directive.templateNamespace`

The property name `type` was too general.
@tbosch tbosch force-pushed the svg-transclusion branch 2 times, most recently from 35a7cab to 623b5ca Compare August 22, 2014 20:28
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants