jkphl / iconizr

A PHP command line tool for converting SVG images to a set of CSS icons (SVG & PNG, single icons and / or CSS sprites) with support for image optimization and Sass output. Created by Joschi Kuphal (@jkphl), licensed under the terms of the MIT license
http://iconizr.com
MIT License
485 stars 36 forks source link

SVGO / Scour are prone to errors #5

Closed jkphl closed 11 years ago

jkphl commented 11 years ago

It turns out that Scour as well as SVGO are prone to errors when served with certain SVG files. At the moment, iconizr fails completely if even one SVG file fails optimizing. This has to be changed immediately.

While I found Scour to be prefered over SVGO (smaller resulting SVG files), it seems to be unmaintained since 2009, so it's unlikely that it's support for problematic SVG files will improve. SVGO could improve over time.

I think I'm going to implement multiple fallbacks: If Scour is available and requested, it will be used for SVG optimization. If it fails, iconizr will look for the presence of SVGO, and use it in case it is available. If even SVGO fails, the unoptimized SVG file will be used.

jkphl commented 11 years ago

Obviously SVGO cannot deal with XML entities like in this case:

<?xml version="1.0" encoding="utf-8"?>
<!-- Generator: Adobe Illustrator 12.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 51448)  -->
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
    <!ENTITY ns_svg "http://www.w3.org/2000/svg">
    <!ENTITY ns_xlink "http://www.w3.org/1999/xlink">
]>
<svg  version="1.1" xmlns="&ns_svg;" xmlns:xlink="&ns_xlink;" width="10" height="10">
    ...
</svg>

Running SVGO on this file leads to an Invalid character entity error. Unfortunately, SVGO doesn't terminate with a proper exit code, so iconizr cannot detect that SVGO failed. Scour doesn't have any issues with the entities. I'm not a node.js guy, but as far is I could find out it seems to be a problem with sax-js(?).

It would probably be the best solution to resolve all entities before passing them on to SVGO. I will take a look into this.

jkphl commented 11 years ago

Fixed with Beta 6