Repository: svg/svgo
Pull request #1147 (2019-08-29 16:33:22)

Fix cleanupids defs last elem
Hi! Today I encountered with unexpected behavior of `cleanupIDs` plugin.

I had an SVG with `<defs>` declared at the end of file, yet still containing another element in front of `<defs>`. But plugin treated this SVG as `hasDefsOnly`.

alexpavlovich: It checks if the whole SVG has only `<defs>` in it. If you do not think this is right, I would appreciate a valid test case in which this check fails. 

Patch:
diff --git a/plugins/cleanupIDs.js b/plugins/cleanupIDs.js
index aebec05..71ffb5f 100644
--- a/plugins/cleanupIDs.js
+++ b/plugins/cleanupIDs.js
@@ -63,11 +63,13 @@ exports.fn = function(data, params) {
                     hasStyleOrScript = true;
                     continue;
                 }
+
                 // Don't remove IDs if the whole SVG consists only of defs.
-                if (item.isElem('defs') && item.parentNode.isElem('svg')) {
+                if (item.isElem('svg')) {
                     var hasDefsOnly = true;
-                    for (var j = i + 1; j < items.content.length; j++) {
-                        if (items.content[j].isElem()) {
+
+                    for (var j = 0; j < item.content.length; j++) {
+                        if (!item.content[j].isElem('defs')) {
                             hasDefsOnly = false;
                             break;
                         }


Test patch:
diff --git a/test/plugins/cleanupIDs.20.svg b/test/plugins/cleanupIDs.20.svg
new file mode 100644
index 0000000..2fa0869
--- /dev/null
+++ b/test/plugins/cleanupIDs.20.svg
@@ -0,0 +1,41 @@
+<svg width="18" height="18" viewBox="0 0 18 18" fill="none" xmlns="http://www.w3.org/2000/svg">
+    <g filter="url(#filter0_dust)">
+        <path d="M2 8a7 7 0 1 1 14 0A7 7 0 0 1 2 8z" fill="#fff"/>
+    </g>
+    <path d="M4 8a5 5 0 1 1 10 0A5 5 0 0 1 4 8z" fill="currentColor"/>
+    <defs>
+        <filter id="filter0_dust" x="0" y="0" width="18" height="18" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+            <feFlood flood-opacity="0" result="BackgroundImageFix"/>
+            <feColorMatrix in="SourceAlpha" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0"/>
+            <feOffset dy="1"/>
+            <feGaussianBlur stdDeviation="1"/>
+            <feColorMatrix values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.2 0"/>
+            <feBlend in2="BackgroundImageFix" result="effect1_dropShadow"/>
+            <feBlend in="SourceGraphic" in2="effect1_dropShadow" result="shape"/>
+        </filter>
+    </defs>
+</svg>
+
+@@@
+
+<svg width="18" height="18" viewBox="0 0 18 18" fill="none" xmlns="http://www.w3.org/2000/svg">
+    <g filter="url(#test__a)">
+        <path d="M2 8a7 7 0 1 1 14 0A7 7 0 0 1 2 8z" fill="#fff"/>
+    </g>
+    <path d="M4 8a5 5 0 1 1 10 0A5 5 0 0 1 4 8z" fill="currentColor"/>
+    <defs>
+        <filter id="test__a" x="0" y="0" width="18" height="18" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+            <feFlood flood-opacity="0" result="BackgroundImageFix"/>
+            <feColorMatrix in="SourceAlpha" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0"/>
+            <feOffset dy="1"/>
+            <feGaussianBlur stdDeviation="1"/>
+            <feColorMatrix values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.2 0"/>
+            <feBlend in2="BackgroundImageFix" result="effect1_dropShadow"/>
+            <feBlend in="SourceGraphic" in2="effect1_dropShadow" result="shape"/>
+        </filter>
+    </defs>
+</svg>
+
+@@@
+
+{"prefix": "test__", "remove": "false"}