RazrFalcon / svgcleaner

svgcleaner could help you to clean up your SVG files from the unnecessary data.
GNU General Public License v2.0
1.62k stars 93 forks source link

Broken output when transforming Debian Openlogo due to 'undefined entity' #240

Open ThomasLamprecht opened 3 years ago

ThomasLamprecht commented 3 years ago

svgcleaner version

Current master, commit 3b64db0071eb460d23d48cabf19dad331ccb1c68

Input:

https://www.debian.org/logos/openlogo-nd.svg

svgcleaner command:

Used the "safe options" I saw a link too in another issue here: https://commons.wikimedia.org/wiki/User:JoKalliauer/Optimization#svgcleaner

svgcleaner openlogo-nd.svg debian-swirl-openlogo.bad.svg --indent 1 --resolve-use no --convert-shapes no --group-by-style no --join-arcto-flags no --join-style-attributes no --remove-comments no --remove-declarations no --remove-invisible-elements no --remove-metadata no --remove-nonsvg-attributes no --remove-nonsvg-elements no --remove-text-attributes no --remove-title no --remove-unreferenced-ids no --trim-ids no --ungroup-groups no --list-separator comma

The tool exits OK, only output I see is: Your image is 22.51% smaller now.

Expected Outcome:

Smaller working SVG

Observed Outcome:

Smaller but broken SVG

When opening this in Firefox 89.0.2

XML Parsing Error: undefined entity
Location: http://192.168.30.59:8000/debian-swirl-openlogo.bad.svg
Line Number 8, Column 5:
    <v:sampleDataSets xmlns:v="&ns_vars;"/>
----^

When opening in Chromium:

This page contains the following errors:
error on line 8 at column 41: Entity 'ns_vars' not defined
Below is a rendering of the page up to the first error.

Did not bother with any further SVG capable viewer, as web is where this would end up anyway.

Manual fixing

The metadata's VariablSets seems to be cleaned up in an broken way, from original:

        <metadata>
                <variableSets  xmlns="&ns_vars;">
                        <variableSet  varSetName="binding1" locked="none">
                                <variables></variables>
                                <v:sampleDataSets  xmlns="&ns_custom;" xmlns:v="&ns_vars;"></v:sampleDataSets>
                        </variableSet>
                </variableSets>
        </metadata>

to belows:

<metadata>
 <variableSets>
  <variableSet varSetName="binding1" locked="none">
   <variables/>
   <v:sampleDataSets xmlns:v="&ns_vars;"/>
  </variableSet>
 </variableSets>

If I delete the line with <v:sampleDataSets xmlns:v="&ns_vars;"/> the SVG is rendered OK again.

RazrFalcon commented 3 years ago

Yes, entities are a bit broken.

JoKalliauer commented 3 years ago

Used the "safe options" I saw a link too in another issue here: https://commons.wikimedia.org/wiki/User:JoKalliauer/Optimization#svgcleaner

This is based on my (i.e. user not developer) input, with "safe options" I mean that it does hardly remove anything that might be useful (for editing).

If you change --remove-nonsvg-elements no to --remove-nonsvg-elements yes it removes the problematic part (ending up with <metadata/> ). As a user, svgcleaner is imho good for valid SVG according to the DTD, so removing of nonsvg-elements will lead to less breakings. (i.e. If you just want small svg-files which don't need to be editable it is more safe to use --remove-nonsvg-elements yes.)

I personally use a different option I use https://github.com/JoKalliauer/cleanupSVG/blob/master/pre.sh#L140: sed -ri "s/=\"([amp38;\#\&\])+ns_vars;\"/=\"http:\/\/ns.adobe.com\/Variables\/1.0\/\"/g" filename.svg that replaces xmlns:v="&ns_vars;" with xmlns:v="http://ns.adobe.com/Variables/1.0/" which is wider supported. After this replacement you can run svgcleaner as before.

PS: Inkscape does also not understand <!ENTITY ns_vars "http://ns.adobe.com/Variables/1.0/"> correctly.

To avoid this issue I recommend to open https://www.debian.org/logos/openlogo-nd.svg in Firefox 89.0.2 (not in Chromium) and press ctrl&s and safe it to disk. this results in Safed_With_FireFox.svg.txt which has the replacement already included.

JoKalliauer commented 3 years ago

Yes, entities are a bit broken.

Why don't you invent --remove-doctype similar as --remove-comments? ( #156 keep Doctype-Declaration ). I think keeping DTD similar to comments might be ok?

RazrFalcon commented 3 years ago

Because you cannot preserve entities. Just like CSS.