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

ngInclude doesn't seem to work on svg elements #5263

Closed
wmertens opened this issue Dec 4, 2013 · 11 comments
Closed

ngInclude doesn't seem to work on svg elements #5263

wmertens opened this issue Dec 4, 2013 · 11 comments

Comments

@wmertens
Copy link

wmertens commented Dec 4, 2013

Plunker: http://plnkr.co/edit/JF8O1NMbdNQLSNYkCNyG?p=preview

Basically, I'd like to use ngInclude to do recursive svg inclusions but that doesn't seem to work.

@caitp
Copy link
Contributor

caitp commented Dec 4, 2013

I might dig into this today if the issue doesn't get resolved/closed before this evening. There seem to be a few issues regarding SVG + angular compilation, so this might be related, but I haven't really dug into it in much detail just yet

@ghost ghost assigned IgorMinar Dec 4, 2013
@IgorMinar
Copy link
Contributor

@wmertens you were missing quotes around the template id: <g ng-include="'svgOrg'"></g>

with quotes ng include does work in svg http://plnkr.co/edit/Igvyeb1g4sWHoeOgAHTz?p=preview

@wmertens
Copy link
Author

wmertens commented Dec 4, 2013

@IgorMinar You're correct, I forgot the single quotes in the plunker but even then it doesn't work for me. What browser are you using?
The second section still doesn't show the org in your plunker on Safari and Chrome (stable) for me. It's supposed to look like the third section.

@wmertens
Copy link
Author

wmertens commented Dec 5, 2013

@caitp did you have a chance to take a look at it? I'd like to get this issue reopened because it's not fixed for me. Can you verify if it works for you on http://plnkr.co/edit/Igvyeb1g4sWHoeOgAHTz?p=preview ?

@caitp
Copy link
Contributor

caitp commented Dec 5, 2013

The W3 validator complains if I use even data-ng-include or data-ng-init in SVG elements.... So I suppose it's not really possible to do it that way (I mean unless this is a different problem, but it does say it's invalid)

Also, it complains about the viewport attribute, which seems to be the case looking at the SVG spec and MDN
doesn't mention it either.

The ngInclude directive isn't being compiled or linked, and I don't think we have any special logic to ignore SVG tags (although it's possible that we do), so it is likely that we just don't see the SVG nodes... I'll see if there's anything I can do about that

edit okay, chrome devtools are acting up a bit in plnkr, it seems ngInclude is being compiled/linked, and the template is compiled as well.hmmm

Issue seems to be at

              currentElement.html(response);
              $animate.enter(currentElement, null, $element, afterAnimation);
              $compile(currentElement.contents())(currentScope);
              currentScope.$emit('$includeContentLoaded');

(https://github.com/angular/angular.js/blob/master/src/ng/directive/ngInclude.js#L203)

The innerHTML gets set correctly, but childNodes are never populated. This isn't specific to jqLite, as it affects jquery 2.0.3 as well


So... I guess theoretically it might be possible to make this work if using a strategy similar to #5235, but it would be really gross and require changes to ngInclude too.

I guess it could be done if there's a serious demand for it, but seen as though it makes markup invalid, that might be an indicator that it's something we shouldn't do

@wmertens
Copy link
Author

wmertens commented Dec 6, 2013

Actually there's a ton of things being used on SVG that probably make markup invalid. ng-repeat works fine in the example and that's not a valid SVG attribute either.

It would be great to fix this, d3.js is awesome but it's very hard to set up the graphing, and being able to ngInclude templates allows you to use d3 for the layout calculations and angular for the binding elements to content. It's much more natural to define a graph in html with bindings than having to do it piecewise in code with the d3.enter().append() etc idiom. To see this illustrated suberbly, watch the http://worrydream.com/#!/DrawingDynamicVisualizationsTalk . Angular isn't a drag and drop thing (yet), but it's sure a lot nicer than the d3 code.

@caitp
Copy link
Contributor

caitp commented Dec 6, 2013

ng-repeat and ng-include don't strictly work the same way (ng-include simply does element.html(theTemplateHTML), and attempts to compile the contents without ensuring that the contents are actually permitted. ng-repeat uses the transclude function instead, which may do a better job of collecting the SVG content...)

Although as far as I can tell, the current $compile/transclude implementation shouldn't be producing valid SVG fragments from SVG content templates, it is more likely to work than the document fragment being generated by jqLite(svgContent)

Interestingly from testing here, http://jsbin.com/UhIrEtES/1/, they seem to both produce valid SVG document fragments


Hmm, actually it generates document fragments, but the document fragments are not SVG document fragments, unless the <svg> tag is at the root of the template. Which is basically what I assumed was happening. I'm not totally sure what the solution to that would be (since there are a lot more svg elements than table elements, a solution like the one I linked above would be harder and nastier to implement)

@wmertens
Copy link
Author

wmertens commented Jan 9, 2014

@caitp I tried taking a page from your book, adding the tag to the svgOrg template as a workaround, but that doesn't seem to make it into an svg fragment. What am I doing wrong, or will this not work?

See http://plnkr.co/edit/is9tr9cPdR3LmbI9scd4?p=preview .

@caitp
Copy link
Contributor

caitp commented Jan 9, 2014

Well @wmertens, there is one fix available: #5508.

Unfortunately it's not a very "friendly" fix, but it's a work-around at least, which is proven to solve the issue when dealing with SVG nodes.

I've previously written a patch designed to do the same thing, working with table nodes, and if that gets merged, then it shouldn't be impossible to do the same sort of thing for SVG and MathML nodes. That's at #5235

I guess I should clarify, they aren't really "available" in the sense of being merged, just available in the sense that solutions exist, and will perhaps be merged in the future

@shock01
Copy link

shock01 commented Jan 10, 2014

This can be fixed by not doing a angular.element().html() in the $compileProvider
but instead use:

  • angular.element(element).html()
  • element.insertAdjacentHtml("afterbegin", content);

That way all svg will be parse properly and interpretted by the browser.

@ghost
Copy link

ghost commented May 21, 2014

http://plnkr.co/edit/Igvyeb1g4sWHoeOgAHTz?p=preview Does not work in Safari 7.0.3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants