vault-development / react-native-svg-uri

Render SVG images in React Native from an URL or static file
849 stars 334 forks source link

Skip unsupported nodes even if they have text value #113

Open skozin opened 6 years ago

skozin commented 6 years ago

This PR fixes #112 and #97. It also removes the code added in #76 to fix #84, as it's no longer needed.

The main cause of both issues is that inspectNode() function fails to exclude children of unsupported node types when they have some value. This leads to nodes of type #text (#84) and #comment (#97, #112) being included into the resulting document as raw text, which, in turn, produces the Raw text cannot be used outside of a <Text> tag error.

Here's what happens: in the very beginning of inspectNode() function, there is a check for inclusion of the node type into ACCEPTED_SVG_ELEMENTS array. If it's not inside the array, null is returned. In a loop inside the same function, there is another check that doesn't include child into the resulting array if recursive call to inspectNode() returned null, so nodes of unsupported types get skipped.

But, inside the same loop, there is a code that adds child node's value into the children array if the node has nodeValue property set. It runs before recursing into inspectNode(), so nodes with nodeValue set get included into resulting array even if their type is not inside ACCEPTED_SVG_ELEMENTS array.

This PR fixes the issue by making sure that the check for whether node type is supported runs for all child nodes, regardless of whether they have nodeValue set.

compojoom commented 6 years ago

Just tested it and it works. Hope that it gets merged soon

rori4 commented 6 years ago

This still has issue with the rendering of text. When it goes to the inspectNode function it returns null because of the check of allowed elements if (!ACCEPTED_SVG_ELEMENTS.includes(node.nodeName)) { return null; } So i still get no render of text svg elements. A quick fix for this would be if (!ACCEPTED_SVG_ELEMENTS.includes(node.nodeName) && !(node.parentNode.localName==="text")) { return null; } I hope this helps someone as I was struggling for 2 hours to understand why i don't get text to render.

skozin commented 6 years ago

@rori4 can you please provide an example of SVG that doesn't render correctly? It will help me fixing this.

HenryQz commented 6 years ago

please merge this commit early. i have a issue relative it 😓

ammichael commented 6 years ago

Why it isn't merged yet? It's causing trouble for bunch of people

machour commented 6 years ago

This doesn't work with this SVG: https://camo.githubusercontent.com/e1d4486cd44174d1ee810a3f2b46ccd5136b6c69/68747470733a2f2f636f766572616c6c732e696f2f6275696c64732f31363235353637372f6261646765