Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize case of SVG elements and attributes in HTML, and preserve accessibility attributes #6593

Merged
merged 4 commits into from Jul 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion flow-libs/posthtml.js.flow
Expand Up @@ -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|},
Expand Down
30 changes: 29 additions & 1 deletion packages/core/integration-tests/test/html.js
Expand Up @@ -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>',
),
);
});
Expand Down Expand Up @@ -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'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fix the colon issue? I notice the benchmarks still fail due to a invalid path with colons and I notice it on other PRs as well. Is that just a bug we introduced somewhere and is now in V2 ? 😟

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should but it seems the test is still failing on windows. I will look into it

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, [
Expand All @@ -2342,6 +2354,7 @@ describe('html', function() {

b = await bundle(
path.join(__dirname, 'integration/url-colon/absolute.html'),
{inputFS: overlayFS},
);

assertBundles(b, [
Expand All @@ -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='));
});
});
@@ -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>
@@ -0,0 +1 @@
<p>Hi</p>

This file was deleted.

3 changes: 2 additions & 1 deletion packages/optimizers/htmlnano/package.json
Expand Up @@ -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"
}
}
75 changes: 73 additions & 2 deletions 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}) {
Expand Down Expand Up @@ -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;
}
102 changes: 102 additions & 0 deletions 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',
};
1 change: 1 addition & 0 deletions packages/transformers/html/src/HTMLTransformer.js
Expand Up @@ -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,
}),
Expand Down