diff --git a/package.json b/package.json index a2d787a8..b05775a8 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "json": "^9.0.6", "lodash.defaultsdeep": "4.6.1", "mkdirp": "^1.0.3", + "mock-require": "^3.0.3", "rimraf": "^3.0.1", "tap": "^11.0.1", "webpack": "^4.8.0", diff --git a/src/svg-renderer.js b/src/svg-renderer.js index a610aa80..6e076284 100644 --- a/src/svg-renderer.js +++ b/src/svg-renderer.js @@ -305,18 +305,27 @@ class SvgRenderer { * @return {number} The largest stroke width in the SVG. */ _findLargestStrokeWidth (rootNode) { + // Per SVG spec, the 'stroke' attribute only applies to shapes and text content elements: + // https://www.w3.org/TR/SVG11/painting.html#StrokeProperty + const STROKABLE_ELEMENTS = new Set([ + // Shape elements (https://www.w3.org/TR/SVG11/intro.html#TermShape) + 'path', 'rect', 'circle', 'ellipse', 'line', 'polyline', 'polygon', + // Text content elements (https://www.w3.org/TR/SVG11/intro.html#TermTextContentElement) + 'altGlyph', 'textPath', 'text', 'tref', 'tspan' + ]); + let largestStrokeWidth = 0; const collectStrokeWidths = domElement => { - if (domElement.getAttribute) { - if (domElement.getAttribute('stroke')) { - largestStrokeWidth = Math.max(largestStrokeWidth, 1); - } - if (domElement.getAttribute('stroke-width')) { - largestStrokeWidth = Math.max( - largestStrokeWidth, - Number(domElement.getAttribute('stroke-width')) || 0 - ); - } + if ( + STROKABLE_ELEMENTS.has(domElement.localName) && + domElement.getAttribute && + domElement.getAttribute('stroke') && + domElement.getAttribute('stroke') !== 'none' + ) { + const strokeWidthAttr = domElement.getAttribute('stroke-width'); + // Stroke width is 1 if unset. + const strokeWidth = Number(strokeWidthAttr) || 1; + largestStrokeWidth = Math.max(largestStrokeWidth, strokeWidth); } for (let i = 0; i < domElement.childNodes.length; i++) { collectStrokeWidths(domElement.childNodes[i]); @@ -381,14 +390,8 @@ class SvgRenderer { // Enlarge the bbox from the largest found stroke width // This may have false-positives, but at least the bbox will always // contain the full graphic including strokes. - // If the width or height is zero however, don't enlarge since - // they won't have a stroke width that needs to be enlarged. - let halfStrokeWidth; - if (bbox.width === 0 || bbox.height === 0) { - halfStrokeWidth = 0; - } else { - halfStrokeWidth = this._findLargestStrokeWidth(this._svgTag) / 2; - } + const halfStrokeWidth = this._findLargestStrokeWidth(this._svgTag) / 2; + const width = bbox.width + (halfStrokeWidth * 2); const height = bbox.height + (halfStrokeWidth * 2); const x = bbox.x - halfStrokeWidth; diff --git a/test/stroke-width.js b/test/stroke-width.js new file mode 100644 index 00000000..b847efc7 --- /dev/null +++ b/test/stroke-width.js @@ -0,0 +1,118 @@ +const test = require('tap').test; +const jsdom = require('jsdom'); +const {JSDOM} = jsdom; + +const mockRequire = require('mock-require'); +// The font inliner uses Webpack loader require syntax, which doesn't work in Node. +mockRequire('../src/font-inliner', () => {}); + +const SvgRenderer = require('../src/svg-renderer'); + +const {window} = new JSDOM(); +// The SvgRenderer constructor tries to get a canvas' context, which doesn't work in JSDOM +window.HTMLCanvasElement.prototype.getContext = () => {}; +global.window = window; +global.document = window.document; +global.DOMParser = window.DOMParser; +const parser = new window.DOMParser(); + +const renderer = new SvgRenderer(); + +const parseSVGString = svgString => parser.parseFromString(svgString, 'image/svg+xml').documentElement; + +test('stroke-width set to maximum', t => { + const svg = parseSVGString(` + + + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 48, 'stroke-width is set to largest value'); + t.end(); +}); + + +test('stroke-width unset', t => { + const svg = parseSVGString(` + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 1, 'stroke-width is 1 by default'); + t.end(); +}); + +test('stroke set to none', t => { + const svg = parseSVGString(` + + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 16, 'stroke="none" doesn\'t affect width'); + t.end(); +}); + +test('stroke-width below 1', t => { + const svg = parseSVGString(` + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 0.5, 'stroke-width is 0.5'); + t.end(); +}); + +test('stroke-width but no stroke', t => { + const svg = parseSVGString(` + + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 16, 'stroke-width is ignored when element has no stroke attribute'); + t.end(); +}); + +test('stroke-width set to invalid value', t => { + const svg = parseSVGString(` + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 1, 'invalid stroke-width defaults to 1'); + t.end(); +}); + +test('stroke-width on the wrong elements', t => { + const svg = parseSVGString(` + + + + + `); + + const largestStrokeWidth = renderer._findLargestStrokeWidth(svg); + + t.equals(largestStrokeWidth, 16, 'stroke-width is ignored when applied to elements that cannot have a stroke'); + t.end(); +});