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.

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
 

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
 ---
 

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/>

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
 ---
 

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
 

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
 

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;

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',
+    },
+  });
+});

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,32 +52,27 @@ 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 !== '' &&
           node.attributes.type !== 'text/css'
         ) {
           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;
         }