Normalize case of SVG elements and attributes in HTML, and preserve a…

…ccessibility attributes (#6593)
parcel-bundler · Jul 17, 2021 · a8a5b70 · a8a5b70
1 parent 940f4fc
commit a8a5b70
Show file tree

Hide file tree

Showing 9 changed files with 227 additions and 6 deletions.
diff --git a/flow-libs/posthtml.js.flow b/flow-libs/posthtml.js.flow
@@ -10,7 +10,7 @@ declare module 'posthtml' {
   declare type PostHTMLNode = {
     tag: string,
     attrs?: {[string]: string, ...},
-    content?: Array<string>,
+    content?: Array<string | PostHTMLNode>,
     location?: {
        start: {|line: number, column: number|},
        end: {|line: number, column: number|},

diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js
@@ -498,7 +498,7 @@ describe('html', function() {
     // minifySvg is false
     assert(
       html.includes(
-        '<svg version="1.1" baseprofile="full" width="300" height="200" xmlns="http://www.w3.org/2000/svg"><rect width="100%" height="100%" fill="red"></rect><circle cx="150" cy="100" r="80" fill="green"></circle><text x="150" y="125" font-size="60" text-anchor="middle" fill="white">SVG</text></svg>',
+        '<svg version="1.1" width="300" height="200" xmlns="http://www.w3.org/2000/svg" baseProfile="full"><rect width="100%" height="100%" fill="red"></rect><circle cx="150" cy="100" r="80" fill="green"></circle><text x="150" y="125" font-size="60" text-anchor="middle" fill="white">SVG</text></svg>',
       ),
     );
   });
@@ -2322,8 +2322,20 @@ describe('html', function() {
   });
 
   it('should work with bundle names that have colons in them', async function() {
+    if (process.platform === 'win32') {
+      return;
+    }
+
+    // Windows paths cannot contain colons and will fail to git clone, so write the file here (in memory).
+    await overlayFS.mkdirp(path.join(__dirname, 'integration/url-colon'));
+    await overlayFS.writeFile(
+      path.join(__dirname, 'integration/url-colon/a:b:c.html'),
+      '<p>Test</p>',
+    );
+
     let b = await bundle(
       path.join(__dirname, 'integration/url-colon/relative.html'),
+      {inputFS: overlayFS},
     );
 
     assertBundles(b, [
@@ -2342,6 +2354,7 @@ describe('html', function() {
 
     b = await bundle(
       path.join(__dirname, 'integration/url-colon/absolute.html'),
+      {inputFS: overlayFS},
     );
 
     assertBundles(b, [
@@ -2358,4 +2371,19 @@ describe('html', function() {
     output = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(output.includes('/a:b:c.html'));
   });
+
+  it('should normalize case of SVG elements and attributes when minified', async function() {
+    let b = await bundle(
+      path.join(__dirname, 'integration/html-svg-case/index.html'),
+      {
+        mode: 'production',
+      },
+    );
+
+    let output = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
+    assert(output.includes('<x-custom stddeviation="0.5"'));
+    assert(output.includes('<svg role="img" viewBox='));
+    assert(output.includes('<filter'));
+    assert(output.includes('<feGaussianBlur in="SourceGraphic" stdDeviation='));
+  });
 });
diff --git a/packages/core/integration-tests/test/integration/html-svg-case/index.html b/packages/core/integration-tests/test/integration/html-svg-case/index.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+  <body>
+    <x-custom stdDeviation="0.5"></x-custom>
+    <svg viewBox="1.8 2.4 2 2" preserveAspectRatio="xMinYMin meet" role="img">
+      <title>Test</title>
+      <defs>
+        <FILTER id="blur">
+          <feGaussianBlur in="SourceGraphic" stdDeviation="0.01" />
+        </FILTER>
+      </defs>
+      <g>
+        <path d="M2 3C3 2 4 3 3 4" stroke-width="0.1" stroke="blue" fill="none" filter="url(#blur)"/>
+      </g>
+    </svg>
+    <a href="other.html">Other</a>
+  </body>
+</html>
diff --git a/packages/core/integration-tests/test/integration/html-svg-case/other.html b/packages/core/integration-tests/test/integration/html-svg-case/other.html
@@ -0,0 +1 @@
+<p>Hi</p>
diff --git a/packages/core/integration-tests/test/integration/url-colon/a:b:c.html b/packages/core/integration-tests/test/integration/url-colon/a:b:c.html
diff --git a/packages/optimizers/htmlnano/package.json b/packages/optimizers/htmlnano/package.json
@@ -23,6 +23,7 @@
     "@parcel/plugin": "2.0.0-beta.3.1",
     "htmlnano": "^1.0.0",
     "nullthrows": "^1.1.1",
-    "posthtml": "^0.16.4"
+    "posthtml": "^0.16.4",
+    "svgo": "^2.3.0"
   }
 }
diff --git a/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js b/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js
@@ -1,9 +1,13 @@
 // @flow strict-local
+import type {PostHTMLNode} from 'posthtml';
 
 import htmlnano from 'htmlnano';
 import {Optimizer} from '@parcel/plugin';
 import posthtml from 'posthtml';
 import path from 'path';
+import {SVG_ATTRS, SVG_TAG_NAMES} from './svgMappings';
+// $FlowFixMe
+import {extendDefaultPlugins} from 'svgo';
 
 export default (new Optimizer({
   async loadConfig({config, options}) {
@@ -34,12 +38,79 @@ export default (new Optimizer({
 
     const htmlNanoConfig = {
       minifyJs: false,
+      minifySvg: {
+        plugins: extendDefaultPlugins([
+          // Copied from htmlnano defaults.
+          {
+            name: 'collapseGroups',
+            active: false,
+          },
+          {
+            name: 'convertShapeToPath',
+            active: false,
+          },
+          // Additional defaults to preserve accessibility information.
+          {
+            name: 'removeTitle',
+            active: false,
+          },
+          {
+            name: 'removeDesc',
+            active: false,
+          },
+          {
+            name: 'removeUnknownsAndDefaults',
+            params: {
+              keepAriaAttrs: true,
+              keepRoleAttr: true,
+            },
+          },
+        ]),
+      },
       ...config,
     };
 
     return {
-      contents: (await posthtml([htmlnano(htmlNanoConfig)]).process(contents))
-        .html,
+      contents: (
+        await posthtml([mapSVG, htmlnano(htmlNanoConfig)]).process(contents)
+      ).html,
     };
   },
 }): Optimizer);
+
+// HTML tags and attributes are case insensitive. The HTML transformer normalizes them so it can
+// more easily process any case. But SVGO requires case sensitive tags and attributes to work correctly.
+// So map lowercased tag and attribute names back to their case-sensitive equivalents.
+function mapSVG(
+  node: string | PostHTMLNode | Array<string | PostHTMLNode>,
+  inSVG = false,
+) {
+  if (Array.isArray(node)) {
+    for (let i = 0; i < node.length; i++) {
+      // $FlowFixMe
+      node[i] = mapSVG(node[i], inSVG);
+    }
+  } else if (node && typeof node === 'object') {
+    let {tag, attrs} = node;
+    if (inSVG || tag === 'svg') {
+      if (SVG_TAG_NAMES[tag]) {
+        node.tag = SVG_TAG_NAMES[tag];
+      }
+
+      if (attrs) {
+        for (let key in attrs) {
+          if (SVG_ATTRS[key]) {
+            attrs[SVG_ATTRS[key]] = attrs[key];
+            delete attrs[key];
+          }
+        }
+      }
+    }
+
+    if (node.content != null) {
+      mapSVG(node.content, inSVG || tag === 'svg');
+    }
+  }
+
+  return node;
+}
diff --git a/packages/optimizers/htmlnano/src/svgMappings.js b/packages/optimizers/htmlnano/src/svgMappings.js
@@ -0,0 +1,102 @@
+// @flow
+// Based on parse5: https://github.com/inikulin/parse5/blob/252819607421a5741cf745bb60c404f023531b0d/packages/parse5/lib/common/foreign-content.js#L54
+
+export const SVG_TAG_NAMES: {|[string]: string|} = {
+  altglyph: 'altGlyph',
+  altglyphdef: 'altGlyphDef',
+  altglyphitem: 'altGlyphItem',
+  animatecolor: 'animateColor',
+  animatemotion: 'animateMotion',
+  animatetransform: 'animateTransform',
+  clippath: 'clipPath',
+  feblend: 'feBlend',
+  fecolormatrix: 'feColorMatrix',
+  fecomponenttransfer: 'feComponentTransfer',
+  fecomposite: 'feComposite',
+  feconvolvematrix: 'feConvolveMatrix',
+  fediffuselighting: 'feDiffuseLighting',
+  fedisplacementmap: 'feDisplacementMap',
+  fedistantlight: 'feDistantLight',
+  feflood: 'feFlood',
+  fefunca: 'feFuncA',
+  fefuncb: 'feFuncB',
+  fefuncg: 'feFuncG',
+  fefuncr: 'feFuncR',
+  fegaussianblur: 'feGaussianBlur',
+  feimage: 'feImage',
+  femerge: 'feMerge',
+  femergenode: 'feMergeNode',
+  femorphology: 'feMorphology',
+  feoffset: 'feOffset',
+  fepointlight: 'fePointLight',
+  fespecularlighting: 'feSpecularLighting',
+  fespotlight: 'feSpotLight',
+  fetile: 'feTile',
+  feturbulence: 'feTurbulence',
+  foreignobject: 'foreignObject',
+  glyphref: 'glyphRef',
+  lineargradient: 'linearGradient',
+  radialgradient: 'radialGradient',
+  textpath: 'textPath',
+};
+
+export const SVG_ATTRS: {|[string]: string|} = {
+  attributename: 'attributeName',
+  attributetype: 'attributeType',
+  basefrequency: 'baseFrequency',
+  baseprofile: 'baseProfile',
+  calcmode: 'calcMode',
+  clippathunits: 'clipPathUnits',
+  diffuseconstant: 'diffuseConstant',
+  edgemode: 'edgeMode',
+  filterunits: 'filterUnits',
+  glyphref: 'glyphRef',
+  gradienttransform: 'gradientTransform',
+  gradientunits: 'gradientUnits',
+  kernelmatrix: 'kernelMatrix',
+  kernelunitlength: 'kernelUnitLength',
+  keypoints: 'keyPoints',
+  keysplines: 'keySplines',
+  keytimes: 'keyTimes',
+  lengthadjust: 'lengthAdjust',
+  limitingconeangle: 'limitingConeAngle',
+  markerheight: 'markerHeight',
+  markerunits: 'markerUnits',
+  markerwidth: 'markerWidth',
+  maskcontentunits: 'maskContentUnits',
+  maskunits: 'maskUnits',
+  numoctaves: 'numOctaves',
+  pathlength: 'pathLength',
+  patterncontentunits: 'patternContentUnits',
+  patterntransform: 'patternTransform',
+  patternunits: 'patternUnits',
+  pointsatx: 'pointsAtX',
+  pointsaty: 'pointsAtY',
+  pointsatz: 'pointsAtZ',
+  preservealpha: 'preserveAlpha',
+  preserveaspectratio: 'preserveAspectRatio',
+  primitiveunits: 'primitiveUnits',
+  refx: 'refX',
+  refy: 'refY',
+  repeatcount: 'repeatCount',
+  repeatdur: 'repeatDur',
+  requiredextensions: 'requiredExtensions',
+  requiredfeatures: 'requiredFeatures',
+  specularconstant: 'specularConstant',
+  specularexponent: 'specularExponent',
+  spreadmethod: 'spreadMethod',
+  startoffset: 'startOffset',
+  stddeviation: 'stdDeviation',
+  stitchtiles: 'stitchTiles',
+  surfacescale: 'surfaceScale',
+  systemlanguage: 'systemLanguage',
+  tablevalues: 'tableValues',
+  targetx: 'targetX',
+  targety: 'targetY',
+  textlength: 'textLength',
+  viewbox: 'viewBox',
+  viewtarget: 'viewTarget',
+  xchannelselector: 'xChannelSelector',
+  ychannelselector: 'yChannelSelector',
+  zoomandpan: 'zoomAndPan',
+};
diff --git a/packages/transformers/html/src/HTMLTransformer.js b/packages/transformers/html/src/HTMLTransformer.js
@@ -18,6 +18,7 @@ export default (new Transformer({
       type: 'posthtml',
       version: '0.4.1',
       program: parse(await asset.getCode(), {
+        lowerCaseTags: true,
         lowerCaseAttributeNames: true,
         sourceLocations: true,
       }),