XML processing instruction

[tkhyn]

Hi !

First, thanks for iconizr which I discovered recently as I have decided to use SVG sprites for the project I am working on. It makes things really easier and is really easily customizable.

However, I run into an issue, which apparently only occurs on windows: PhantomJS will hang forever if there is no XML processing instruction in the file. See ariya/phantomjs#12076 and domenic/svg2png#11.

To make it work by the time PhantomJS is fixed, I attempted to change the SVGO options through the cleanconfig option, disabling the removeXMLProcInst plugin.
That was ok for the pre-processed individual files, but not for the SVG sprite file itself, where the processing instruction was still missing.

I had a look at the code in svg-sprite, which actually generates the SVG sprite, and attempted to modify it so that it could add the missing line at the top when the removeXMLProcInst plugin was disabled by the user.

My attempt is available here: tkhyn@bdef274

I did not know anything about node.js 2 days ago and did not have the time to understand every aspect of how svg-sprite was working so there may be better ways to do the same thing, but this change makes things work for me. Through this report I mostly wanted to raise the issue and offer a possible solution. A similar approach may be used for the removeDoctype SVGO plugin (it does not look like any other plugin would require the same treatment).

Something worth noting but still about that: I modified the SVGObj.toSVG method so that the XML processing instruction is only removed when inline is true, and kept otherwise.

Thanks again,

Thomas

[jkphl]

Hi @tkhyn,

thanks for your kind words and your report! At first glance, your modifications look pretty reasonable (btw, I'm not at all a Node.js pro as well ;)) and I'll review them as soon as I find some minutes time (hopefully on Sunday?!). By the way, I only stripped the XML declaration to save some bytes ... ;)

Cheers,
Joschi

[tkhyn]

By the way, I only stripped the XML declaration to save some bytes ... ;)

Yes, indeed, this block should not be included when inline is false and removeXMLProcInst is enabled.

Here is a slightly cleaner approach (basically moved the removeXMLProcInst and removeDocType detection code in the SVGSprite constructor, where I add properties to the config).

tkhyn@b045574

I won't do a pull request as I have unsuccessfully tried to run the mocha tests. Looks like there are really too many issues with node.js and node-phantomjs on windows and I don't have the time to solve them (nothing wrong with svg-sprite itself but more with node.js, I'll definitely stick with python and fab whenever possible in the future!).

[jkphl]

Hey @tkhyn,

I implemented the desired functionality now, although I took a slightly different (more compact) approach. Could you please check if this works for you now? I'll try to publish a new release later today, until then you could use the GitHub version. Please feel free to reopen this issue in case of problems.

Thanks & cheers,
Joschi

[tkhyn]

Confirming it works for me. Thanks!

[jkphl]

Good to hear! You're welcome. :)