Cleanup style attributes

lib/css.js

@@ -1,5 +1,6 @@
 /**
  * @typedef {import('./types.js').XastElement} XastElement
+ * @typedef {XastElement&{style?:string,declarations?:import('./types.js').CSSDeclarationMap}} XastElementExtended
  * @typedef {import('./types.js').CSSFeatures} CSSFeatures
  * @typedef{{type:'AttributeSelector',name:string,matcher:string|null,value:string|null}} AttributeSelector
  * @typedef{{type:'ClassSelector',name:string}} ClassSelector
@@ -310,6 +311,23 @@ export class CSSParseError extends Error {
   }
 }
 
+/**
+ * @param {XastElementExtended} element
+ * @returns {import('./types.js').CSSDeclarationMap|undefined}
+ */
+export function getStyleDeclarations(element) {
+  const style = element.attributes.style;
+  if (style === undefined || style === '') {
+    return;
+  }
+  if (element.style !== style) {
+    element.style = style;
+    element.declarations = parseStyleDeclarations(style);
+  }
+  // Copy cached map in case it is changed by caller.
+  return new Map(element.declarations);
+}
+
 /**
  * @param {string} css
  */
@@ -319,9 +337,10 @@ export function _isStyleComplex(css) {
 
 /**
  * @param {string|undefined} css
- * @returns {Map<string,string>}
+ * @returns {Map<string,import('./types.js').CSSPropertyValue>}
  */
 export function parseStyleDeclarations(css) {
+  /** @type {Map<string,import('./types.js').CSSPropertyValue>} */
   const declarations = new Map();
   if (css === undefined) {
     return declarations;
@@ -337,9 +356,35 @@ export function parseStyleDeclarations(css) {
     if (declaration) {
       const pv = declaration.split(':');
       if (pv.length === 2) {
-        declarations.set(pv[0].trim(), pv[1].trim());
+        const dec = pv[1].trim();
+        const value = dec.endsWith('!important')
+          ? { value: dec.substring(0, dec.length - 10).trim(), important: true }
+          : { value: dec, important: false };
+        declarations.set(pv[0].trim(), value);
       }
     }
   }
   return declarations;
 }
+
+/**
+ * @param {import('../lib/types.js').XastElement} element
+ * @param {Map<string,{value:string,important?:boolean}>} properties
+ */
+export function writeStyleAttribute(element, properties) {
+  let style = '';
+  for (const [p, decValue] of properties.entries()) {
+    if (style !== '') {
+      style += ';';
+    }
+    style += `${p}:${decValue.value}`;
+    if (decValue.important) {
+      style += '!important';
+    }
+  }
+  if (style) {
+    element.attributes.style = style;
+  } else {
+    delete element.attributes.style;
+  }
+}

lib/docdata.js

@@ -7,7 +7,7 @@ import {
 import { parseStylesheet } from './style-css-tree.js';
 import { findReferences } from './svgo/tools.js';
 import { detachNodeFromParent, visit, visitSkip } from './xast.js';
-import { CSSParseError, parseStyleDeclarations } from './css.js';
+import { CSSParseError, getStyleDeclarations } from './css.js';
 
 /**
  * @typedef {import('../lib/types.js').XastElement} XastElement
@@ -83,6 +83,7 @@ export class StyleData {
    * @returns {Map<string,string|null>}
    */
   computeOwnStyle(node) {
+    /** @type {Map<string,string|null>} */
     const computedStyles = new Map();
 
     if (node.type === 'root') {
@@ -120,11 +121,14 @@ export class StyleData {
     }
 
     // Override with inline styles.
-    parseStyleDeclarations(node.attributes.style).forEach((value, name) => {
-      if (!importantProperties.has(name)) {
-        computedStyles.set(name, value);
-      }
-    });
+    const declarations = getStyleDeclarations(node);
+    if (declarations) {
+      declarations.forEach((value, name) => {
+        if (value.important || !importantProperties.has(name)) {
+          computedStyles.set(name, value.value);
+        }
+      });
+    }
 
     return computedStyles;
   }

lib/style-css-tree-tools.js

@@ -2,9 +2,10 @@ import * as csstree from 'css-tree';
 
 /**
  * @param {string|undefined} css
- * @returns {Map<string,string>}
+ * @returns {Map<string,import('./types.js').CSSPropertyValue>}
  */
 export function _parseStyleDeclarations(css) {
+  /** @type {Map<string,import('./types.js').CSSPropertyValue>} */
   const declarations = new Map();
   if (css === undefined) {
     return declarations;
@@ -15,10 +16,10 @@ export function _parseStyleDeclarations(css) {
   });
   csstree.walk(ast, (cssNode) => {
     if (cssNode.type === 'Declaration') {
-      declarations.set(
-        cssNode.property.toLowerCase(),
-        csstree.generate(cssNode.value),
-      );
+      declarations.set(cssNode.property.toLowerCase(), {
+        value: csstree.generate(cssNode.value),
+        important: !!cssNode.important,
+      });
     }
   });
   return declarations;

lib/style-css-tree.js

@@ -209,9 +209,10 @@ function createCSSSelector(node) {
 
 /**
  * @param {csstree.CssNode} ast
- * @returns {Map<string,{value:string,important?:boolean}>}
+ * @returns {Map<string,import('./types.js').CSSPropertyValue>}
  */
 export function parseStyleSheetDeclarations(ast) {
+  /** @type {Map<string,import('./types.js').CSSPropertyValue>} */
   const declarations = new Map();
   csstree.walk(ast, (cssNode) => {
     switch (cssNode.type) {
@@ -221,7 +222,7 @@ export function parseStyleSheetDeclarations(ast) {
       case 'Declaration':
         declarations.set(cssNode.property.toLowerCase(), {
           value: csstree.generate(cssNode.value),
-          important: cssNode.important,
+          important: !!cssNode.important,
         });
         break;
       case 'Raw':

lib/style.js

@@ -277,26 +277,3 @@ export const computeStyle = (stylesheet, node) => {
   }
   return computedStyles;
 };
-
-/**
- * @param {import('../lib/types.js').XastElement} element
- * @param {Map<string,string|{value:string,important?:boolean}>} properties
- */
-export function writeStyleAttribute(element, properties) {
-  let style = '';
-  for (const [p, v] of properties.entries()) {
-    if (style !== '') {
-      style += ';';
-    }
-    if (typeof v === 'string') {
-      style += `${p}:${v}`;
-    } else {
-      style += `${p}:${v.value}`;
-    }
-  }
-  if (style) {
-    element.attributes.style = style;
-  } else {
-    delete element.attributes.style;
-  }
-}

lib/types.d.ts

@@ -162,6 +162,10 @@ export class CSSRule {
   matches(element: XastElement): boolean;
 }
 
+export type CSSPropertyValue = { value: string; important: boolean };
+
+export type CSSDeclarationMap = Map<string, CSSPropertyValue>;
+
 export type PluginInfo = {
   path?: string;
   passNumber: number;

plugins/cleanupStyleAttributes.js

@@ -1,5 +1,4 @@
-import { parseStyleDeclarations } from '../lib/css.js';
-import { writeStyleAttribute } from '../lib/style.js';
+import { getStyleDeclarations, writeStyleAttribute } from '../lib/css.js';
 import { visitSkip } from '../lib/xast.js';
 import { elemsGroups, uselessShapeProperties } from './_collections.js';
 
@@ -170,8 +169,8 @@ export const fn = (root, params, info) => {
           return;
         }
 
-        const origStyle = node.attributes.style;
-        if (origStyle === undefined || origStyle === '') {
+        const origProperties = getStyleDeclarations(node);
+        if (!origProperties) {
           return;
         }
 
@@ -184,7 +183,6 @@ export const fn = (root, params, info) => {
         }
 
         const isShapeGroup = node.name === 'g' && hasOnlyShapeChildren(node);
-        const origProperties = parseStyleDeclarations(origStyle);
         for (const [p, v] of origProperties.entries()) {
           if (!elementCanHaveProperty(node.name, p)) {
             continue;

plugins/inlineStyles.js

@@ -1,11 +1,11 @@
+import { writeStyleAttribute } from '../lib/css.js';
+
 /**
  * @typedef {import('../lib/types.js').XastElement} XastElement
  * @typedef {import('../lib/types.js').XastParent} XastParent
  * @typedef {import('../lib/types.js').CSSRule} CSSRule
  */
 
-import { writeStyleAttribute } from '../lib/style.js';
-
 export const name = 'inlineStyles';
 export const description =
   'Move properties in <style> elements to style attributes';

plugins/moveElemsStylesToGroup.js

@@ -1,5 +1,4 @@
-import { parseStyleDeclarations } from '../lib/css.js';
-import { writeStyleAttribute } from '../lib/style.js';
+import { getStyleDeclarations, writeStyleAttribute } from '../lib/css.js';
 import { inheritableAttrs } from './_collections.js';
 
 export const name = 'moveElemsStylesToGroup';
@@ -28,12 +27,12 @@ export const fn = (root, params, info) => {
         }
 
         // Record child properties so we don't have to re-parse them.
-        /** @type {Map<import('../lib/types.js').XastElement,Map<string,string>>} */
+        /** @type {Map<import('../lib/types.js').XastElement,Map<string,{value:string,important?:boolean}>>} */
         const childProperties = new Map();
 
         /**
          * Find common properties in group children.
-         * @type {Map<string, string>}
+         * @type {Map<string, {value:string,important?:boolean}>}
          */
         const commonProperties = new Map();
         let initial = true;
@@ -42,11 +41,10 @@ export const fn = (root, params, info) => {
             continue;
           }
 
-          const style = child.attributes.style;
-          if (style === undefined) {
+          const properties = getStyleDeclarations(child);
+          if (properties === undefined) {
             return;
           }
-          const properties = parseStyleDeclarations(style);
           childProperties.set(child, properties);
 
           if (initial) {
@@ -63,7 +61,12 @@ export const fn = (root, params, info) => {
           } else {
             // exclude uncommon attributes from initial list
             for (const [name, value] of commonProperties) {
-              if (properties.get(name) !== value) {
+              const dec = properties.get(name);
+              if (
+                !dec ||
+                dec.value !== value.value ||
+                dec.important !== value.important
+              ) {
                 commonProperties.delete(name);
               }
             }
@@ -85,7 +88,8 @@ export const fn = (root, params, info) => {
         }
 
         // Add common child properties to group.
-        const groupProperties = parseStyleDeclarations(node.attributes.style);
+        /** @type {Map<string,{value:string,important?:boolean}>} */
+        const groupProperties = getStyleDeclarations(node) ?? new Map();
 
         for (const [name, value] of commonProperties) {
           groupProperties.set(name, value);
@@ -96,8 +100,8 @@ export const fn = (root, params, info) => {
         // Delete common properties from children.
         for (const child of node.children) {
           if (child.type === 'element') {
-            /** @type {Map<string,string>} */
-            // @ts-ignore
+            /** @type {Map<string,{value:string,important?:boolean}>} */
+            // @ts-ignore - properties should be defined because
             const properties = childProperties.get(child);
             for (const [name] of commonProperties) {
               properties.delete(name);

test/lib/css.parsestyledata.test.js

@@ -29,31 +29,54 @@ describe('test whether a style attribute has complex declarations', function ()
 });
 
 describe('test parsing of style attributes', function () {
+  /** @type{{input:string,expected:Object<string,import('../../lib/types.js').CSSPropertyValue>}[]
+  } */
   const tests = [
     {
       input: 'fill:red;stroke:green',
-      expected: { fill: 'red', stroke: 'green' },
+      expected: {
+        fill: { value: 'red', important: false },
+        stroke: { value: 'green', important: false },
+      },
+    },
+    {
+      input: 'fill:red!important;stroke:green',
+      expected: {
+        fill: { value: 'red', important: true },
+        stroke: { value: 'green', important: false },
+      },
     },
     {
       input: ' ; fill: red  ; stroke : green   ;  ',
-      expected: { fill: 'red', stroke: 'green' },
+      expected: {
+        fill: { value: 'red', important: false },
+        stroke: { value: 'green', important: false },
+      },
     },
     {
       input: 'mask: url(masks.svg#star) stroke-box;',
-      expected: { mask: 'url(masks.svg#star) stroke-box' },
+      expected: {
+        mask: { value: 'url(masks.svg#star) stroke-box', important: false },
+      },
     },
     {
       input: 'mask: url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box;',
       expected: {
-        mask: 'url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box',
+        mask: {
+          value: 'url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box',
+          important: false,
+        },
       },
     },
     {
       input:
         'mask: url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box;fill:red ',
       expected: {
-        mask: 'url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box',
-        fill: 'red',
+        mask: {
+          value: 'url(http://localhost//test.svg?x=1;y=2#mask1) stroke-box',
+          important: false,
+        },
+        fill: { value: 'red', important: false },
       },
     },
   ];
@@ -64,7 +87,8 @@ describe('test parsing of style attributes', function () {
       const parsed = parseStyleDeclarations(test.input);
       expect(parsed.size).toBe(Object.keys(test.expected).length);
       for (const [prop, value] of Object.entries(test.expected)) {
-        expect(parsed.get(prop)).toBe(value);
+        expect(parsed.get(prop)?.value).toBe(value.value);
+        expect(parsed.get(prop)?.important).toBe(value.important);
       }
     });
   }