feat(inlineStyles): remove redundant presentation attrs (#1829)

SVGs can have the same presentation attribute declared redundantly in both the node attributes and `<style>` tag. This wouldn't break anything, but we can shave off a few more bytes by dropping the attribute in this case.
2026-01-27 07:02:06 +03:00 · 2023-11-11 12:43:13 +00:00
parent 742cee1910
commit 4c2cc1bbdb
11 changed files with 144 additions and 37 deletions
--- a/docs/03-plugins/inline-styles.mdx
+++ b/docs/03-plugins/inline-styles.mdx
@@ -16,7 +16,7 @@ svgo:
      description: What pseudo-classes and pseudo-elements to use. An empty string signifies all non-pseudo-classes and non-pseudo-elements. 
 ---

-Move and merge styles from `<style>` elements to a respective elements `style` attributes.
+Merges styles from `<style>` elements to the `style` attribute of matching elements.

 ## Usage

--- a/docs/03-plugins/remove-attributes-by-selector.mdx
+++ b/docs/03-plugins/remove-attributes-by-selector.mdx
@@ -4,7 +4,7 @@ svgo:
  pluginId: removeAttributesBySelector
  parameters:
    selectors:
-      description: This is an array of objects with two properties, <code>selector</code>, and <code>attributes</code>, which represent a CSS selector and the attributes to remove respectively.
+      description: An array of objects with two properties, <code>selector</code>, and <code>attributes</code>, which represent a CSS selector and the attributes to remove respectively.
      default: null
 ---

--- a/docs/03-plugins/remove-comments.mdx
+++ b/docs/03-plugins/remove-comments.mdx
@@ -28,6 +28,10 @@ Removing a comment like this may be considered a breach of the license terms, as

 <PluginUsage/>

+### Parameters
+
+<PluginParams/>
+
 ## Demo

 <PluginDemo/>
--- a/docs/03-plugins/remove-empty-text.mdx
+++ b/docs/03-plugins/remove-empty-text.mdx
@@ -5,13 +5,13 @@ svgo:
  defaultPlugin: true
  parameters:
    text:
-      description: Removes empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/text" target="_blank"><code>&lt;text&gt;</code></a> elements.
+      description: If to remove empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/text" target="_blank"><code>&lt;text&gt;</code></a> elements.
      default: true
    tspan:
-      description: Removes empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/tspan" target="_blank"><code>&lt;tspan&gt;</code></a> elements.
+      description: If to remove empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/tspan" target="_blank"><code>&lt;tspan&gt;</code></a> elements.
      default: true
    tref:
-      description: Removes empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/tref" target="_blank"><code>&lt;tref&gt;</code></a> elements.
+      description: If to remove empty <a href="https://developer.mozilla.org/docs/Web/SVG/Element/tref" target="_blank"><code>&lt;tref&gt;</code></a> elements.
      default: true
 ---

--- a/docs/03-plugins/remove-hidden-elems.mdx
+++ b/docs/03-plugins/remove-hidden-elems.mdx
@@ -55,7 +55,7 @@ Remove hidden or invisible elements from the document. This can be elements with

 This plugin ignores non-rendering elements, such as [`<clipPath>`](https://developer.mozilla.org/docs/Web/SVG/Element/clipPath) and [`<linearGradient>`](https://developer.mozilla.org/docs/Web/SVG/Element/linearGradient), which still apply regardless of styles, unless they are unused.

-Refer to the paremeters for the conditions this plugin looks for. All checks enabled by default.
+Refer to the parameters for the conditions this plugin looks for. All checks enabled by default.

 ## Usage

--- a/docs/03-plugins/remove-useless-stroke-and-fill.mdx
+++ b/docs/03-plugins/remove-useless-stroke-and-fill.mdx
@@ -17,7 +17,7 @@ svgo:

 Removes useless `stroke` and `fill` attributes.

-Assigning these attributes can sometimes change nothing in the document. For example, in most cases assigning a `stoke` color is redundant if the elements [`stroke-width`](https://developer.mozilla.org/docs/Web/SVG/Attribute/stroke-width) or [`stroke-opacity`](https://developer.mozilla.org/docs/Web/SVG/Attribute/stroke-opacity) is `0`.
+Assigning these attributes can sometimes change nothing in the document. For example, in most cases assigning a `stroke` color is redundant if the elements [`stroke-width`](https://developer.mozilla.org/docs/Web/SVG/Attribute/stroke-width) or [`stroke-opacity`](https://developer.mozilla.org/docs/Web/SVG/Attribute/stroke-opacity) is `0`.

 ## Usage

--- a/lib/style.js
+++ b/lib/style.js
@@ -276,3 +276,22 @@ const computeStyle = (stylesheet, node) => {
  return computedStyles;
 };
 exports.computeStyle = computeStyle;
+
+/**
+ * Determines if the CSS selector references the given attribute.
+ *
+ * @param {csstree.ListItem<csstree.CssNode>|string} selector
+ * @param {string} attr
+ * @returns {boolean}
+ * @see https://developer.mozilla.org/docs/Web/CSS/Attribute_selectors
+ */
+const includesAttrSelector = (selector, attr) => {
+  const attrSelectorPattern = new RegExp(`\\[\\s*${attr}\\s*[\\]=~|^$*]`, 'i');
+
+  if (typeof selector === 'string') {
+    return attrSelectorPattern.test(attr);
+  }
+
+  return attrSelectorPattern.test(csstree.generate(selector.data));
+};
+exports.includesAttrSelector = includesAttrSelector;
--- a/lib/style.test.js
+++ b/lib/style.test.js
@@ -306,3 +306,32 @@ it('ignores keyframes atrule', () => {
    },
  });
 });
+
+it('ignores @-webkit-keyframes atrule', () => {
+  const root = parseSvg(`
+      <svg>
+        <style>
+          .a {
+            animation: loading 4s linear infinite;
+          }
+          @-webkit-keyframes loading {
+            0% {
+              stroke-dashoffset: 440;
+            }
+            50% {
+              stroke-dashoffset: 0;
+            }
+          }
+        </style>
+        <rect id="element" class="a" />
+      </svg>
+    `);
+  const stylesheet = collectStylesheet(root);
+  expect(computeStyle(stylesheet, getElementById(root, 'element'))).toEqual({
+    animation: {
+      type: 'static',
+      inherited: false,
+      value: 'loading 4s linear infinite',
+    },
+  });
+});
--- a/plugins/inlineStyles.js
+++ b/plugins/inlineStyles.js
@@ -1,7 +1,6 @@
 'use strict';

 /**
- * @typedef {import('../lib/types').Specificity} Specificity
 * @typedef {import('../lib/types').XastElement} XastElement
 * @typedef {import('../lib/types').XastParent} XastParent
 */
@@ -16,7 +15,8 @@ const {
  querySelectorAll,
  detachNodeFromParent,
 } = require('../lib/xast.js');
-const { compareSpecificity } = require('../lib/style');
+const { compareSpecificity, includesAttrSelector } = require('../lib/style');
+const { attrsGroups } = require('./_collections');

 exports.name = 'inlineStyles';
 exports.description = 'inline styles (additional options)';
@@ -52,15 +52,12 @@ exports.fn = (root, params) => {
  return {
    element: {
      enter: (node, parentNode) => {
-        // skip <foreignObject /> content
        if (node.name === 'foreignObject') {
          return visitSkip;
        }
-        // collect only non-empty <style /> elements
        if (node.name !== 'style' || node.children.length === 0) {
          return;
        }
-        // values other than the empty string or text/css are not used
        if (
          node.attributes.type != null &&
          node.attributes.type !== '' &&
@@ -68,16 +65,14 @@ exports.fn = (root, params) => {
        ) {
          return;
        }
-        // parse css in style element
-        let cssText = '';
-        for (const child of node.children) {
-          if (child.type === 'text' || child.type === 'cdata') {
-            cssText += child.value;
-          }
-        }
-        /**
-         * @type {?csstree.CssNode}
-         */
+
+        const cssText = node.children
+          .filter((child) => child.type === 'text' || child.type === 'cdata')
+          // @ts-ignore
+          .map((child) => child.value)
+          .join('');
+
+        /** @type {?csstree.CssNode} */
        let cssAst = null;
        try {
          cssAst = csstree.parse(cssText, {
@@ -102,14 +97,14 @@ exports.fn = (root, params) => {
            }

            // skip media queries not included into useMqs param
-            let mq = '';
+            let mediaQuery = '';
            if (atrule != null) {
-              mq = atrule.name;
+              mediaQuery = atrule.name;
              if (atrule.prelude != null) {
-                mq += ` ${csstree.generate(atrule.prelude)}`;
+                mediaQuery += ` ${csstree.generate(atrule.prelude)}`;
              }
            }
-            if (useMqs.includes(mq) === false) {
+            if (!useMqs.includes(mediaQuery)) {
              return;
            }

@@ -138,7 +133,8 @@ exports.fn = (root, params) => {
                pseudos.map((pseudo) => pseudo.item.data)
              ),
            });
-            if (usePseudos.includes(pseudoSelectors) === false) {
+
+            if (!usePseudos.includes(pseudoSelectors)) {
              return;
            }

@@ -160,8 +156,8 @@ exports.fn = (root, params) => {
        if (styles.length === 0) {
          return;
        }
-        // stable sort selectors
-        const sortedSelectors = [...selectors]
+        const sortedSelectors = selectors
+          .slice()
          .sort((a, b) => {
            const aSpecificity = specificity(a.item.data);
            const bSpecificity = specificity(b.item.data);
@@ -221,9 +217,18 @@ exports.fn = (root, params) => {
                // no inline styles, external styles,                                    external styles used
                // inline styles,    external styles same   priority as inline styles,   inline   styles used
                // inline styles,    external styles higher priority than inline styles, external styles used
-                const matchedItem = styleDeclarationItems.get(
-                  ruleDeclaration.property
-                );
+                const property = ruleDeclaration.property;
+
+                if (
+                  attrsGroups.presentation.includes(property) &&
+                  !selectors.some((selector) =>
+                    includesAttrSelector(selector.item, property)
+                  )
+                ) {
+                  delete selectedEl.attributes[property];
+                }
+
+                const matchedItem = styleDeclarationItems.get(property);
                const ruleDeclarationItem =
                  styleDeclarationList.children.createItem(ruleDeclaration);
                if (matchedItem == null) {
@@ -236,10 +241,7 @@ exports.fn = (root, params) => {
                    matchedItem,
                    ruleDeclarationItem
                  );
-                  styleDeclarationItems.set(
-                    ruleDeclaration.property,
-                    ruleDeclarationItem
-                  );
+                  styleDeclarationItems.set(property, ruleDeclarationItem);
                }
              },
            });
@@ -262,7 +264,7 @@ exports.fn = (root, params) => {
        }

        // no further processing required
-        if (removeMatchedSelectors === false) {
+        if (!removeMatchedSelectors) {
          return;
        }

--- a/test/plugins/inlineStyles.24.svg
+++ b/test/plugins/inlineStyles.24.svg
@@ -0,0 +1,21 @@
+If we're going to inline styles for property that is also a presentation
+attribute, and that presentation attribute was already defined in the node, we
+can just drop the presentation attribute as it would be overridden by the style
+anyway.
+
+===
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 269 349">
+  <style type="text/css">
+    .a {
+      fill: #059669;
+    }
+  </style>
+  <path class="a" d="M191.5,324.1V355l9.6-31.6A77.49,77.49,0,0,1,191.5,324.1Z" fill="#059669" transform="translate(-57.17 -13.4)"/>
+</svg>
+
+@@@
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 269 349">
+    <path d="M191.5,324.1V355l9.6-31.6A77.49,77.49,0,0,1,191.5,324.1Z" transform="translate(-57.17 -13.4)" style="fill:#059669"/>
+</svg>
--- a/test/plugins/inlineStyles.25.svg
+++ b/test/plugins/inlineStyles.25.svg
@@ -0,0 +1,32 @@
+Don't remove the redundant presentation attribute if it's used in a CSS
+selector in a `<style> tag.
+
+===
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 50">
+  <style>
+    .a {
+      stroke: red;
+    }
+
+    [stroke] + path {
+      stroke: purple;
+    }
+  </style>
+  <path class="a" d="M10 10h20" stroke="red"/>
+  <path d="M10 20h20"/>
+  <path d="M10 30h20" stroke="yellow"/>
+  <path d="M10 40h20"/>
+</svg>
+
+@@@
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 50">
+    <style>
+        [stroke]+path{stroke:purple}
+    </style>
+    <path d="M10 10h20" stroke="red" style="stroke:red"/>
+    <path d="M10 20h20"/>
+    <path d="M10 30h20" stroke="yellow"/>
+    <path d="M10 40h20"/>
+</svg>