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.

Sign up for GitHub

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

Jump to bottom

feat(removeDeprecatedAttrs): new removeDeprecatedAttrs plugin #1869

Merged
merged 1 commit into from
Jan 15, 2024
Merged

feat(removeDeprecatedAttrs): new removeDeprecatedAttrs plugin #1869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/03-plugins/remove-deprecated-attrs.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
title: Remove Deprecated Attributes
svgo:
pluginId: removeDeprecatedAttrs
defaultPlugin: true
jdufresne marked this conversation as resolved.
Show resolved Hide resolved
parameters:
removeAny:
description: By default, this plugin only removes safe deprecated attributes that do not change the rendered image. Enabling this will remove all deprecated attributes which may impact rendering.
type: boolean
default: false
---

Removes deprecated attributes from elements in the document.

This plugin does not remove attributes from the deprecated XLink namespace. To remove them, use the [Remove XLink](/docs/plugins/remove-xlink/) plugin.

## Usage

<PluginUsage />

## Demo

<PluginDemo />

## Implementation

- https://github.com/svg/svgo/blob/main/plugins/removeDeprecatedAttrs.js
2 changes: 2 additions & 0 deletions lib/builtin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import * as prefixIds from '../plugins/prefixIds.js';
import * as removeAttributesBySelector from '../plugins/removeAttributesBySelector.js';
import * as removeAttrs from '../plugins/removeAttrs.js';
import * as removeComments from '../plugins/removeComments.js';
import * as removeDeprecatedAttrs from '../plugins/removeDeprecatedAttrs.js';
import * as removeDesc from '../plugins/removeDesc.js';
import * as removeDimensions from '../plugins/removeDimensions.js';
import * as removeDoctype from '../plugins/removeDoctype.js';
Expand Down Expand Up @@ -79,6 +80,7 @@ export const builtin = [
removeAttributesBySelector,
removeAttrs,
removeComments,
removeDeprecatedAttrs,
removeDesc,
removeDimensions,
removeDoctype,
Expand Down
102 changes: 102 additions & 0 deletions plugins/_collections.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,11 +375,35 @@ export const attrsGroupsDefaults = {
},
};

/**
* @type {Record<string, { safe?: Set<string>, unsafe?: Set<string> }>}
* @see https://www.w3.org/TR/SVG11/intro.html#Definitions
*/
export const attrsGroupsDeprecated = {
animationAttributeTarget: { unsafe: new Set(['attributeType']) },
conditionalProcessing: { unsafe: new Set(['requiredFeatures']) },
core: { unsafe: new Set(['xml:base', 'xml:lang', 'xml:space']) },
presentation: {
Copy link
Member

@SethFalco SethFalco Dec 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Presentation attributes can be used in styles as well. We should probably be removing them too.

Do you have time to add a check, where if the attribute being removed is a presentation attribute, also parse the style attribute and remove instances from there too. If the style attribute becomes empty, delete the style attribute entirely.

I worked on something like this recently in the cleanupEnableBackground plugin, so you can use it as a reference:

Basically, if the node defines the style attribute, parse the styles and if it's a DeclarationList, iterate the list and remove unwanted styles. Finally, you can generate the new styles with csstree.generate and save it back, or delete the attribute if it's become empty.

Instead of collecting the declarations like I did, you can just remove them immediately from the list from inside css.walk I think. 🤔

Important to note that the same attribute can be redundantly defined more than once in CSS, so it's worth looping through all styles and not exit early if the attribute is found.

It could be worth updating the stylesheet (<style> tags) too.

unsafe: new Set([
'clip',
'color-profile',
'enable-background',
'glyph-orientation-horizontal',
'glyph-orientation-vertical',
'kerning',
]),
},
};

/**
* @type {Record<string, {
* attrsGroups: Set<string>,
* attrs?: Set<string>,
* defaults?: Record<string, string>,
* deprecated?: {
* safe?: Set<string>,
* unsafe?: Set<string>,
* },
* contentGroups?: Set<string>,
* content?: Set<string>,
* }>}
Expand Down Expand Up @@ -574,6 +598,7 @@ export const elems = {
name: 'sRGB',
'rendering-intent': 'auto',
},
deprecated: { unsafe: new Set(['name']) },
contentGroups: new Set(['descriptive']),
},
cursor: {
Expand Down Expand Up @@ -958,6 +983,7 @@ export const elems = {
width: '120%',
height: '120%',
},
deprecated: { unsafe: new Set(['filterRes']) },
contentGroups: new Set(['descriptive', 'filterPrimitive']),
content: new Set(['animate', 'set']),
},
Expand All @@ -978,6 +1004,15 @@ export const elems = {
'horiz-origin-x': '0',
'horiz-origin-y': '0',
},
deprecated: {
unsafe: new Set([
'horiz-origin-x',
'horiz-origin-y',
'vert-adv-y',
'vert-origin-x',
'vert-origin-y',
]),
},
contentGroups: new Set(['descriptive']),
content: new Set(['font-face', 'glyph', 'hkern', 'missing-glyph', 'vkern']),
},
Expand Down Expand Up @@ -1028,6 +1063,31 @@ export const elems = {
'panose-1': '0 0 0 0 0 0 0 0 0 0',
slope: '0',
},
deprecated: {
unsafe: new Set([
'accent-height',
'alphabetic',
'ascent',
'bbox',
'cap-height',
'descent',
'hanging',
'ideographic',
'mathematical',
'panose-1',
'slope',
'stemh',
'stemv',
'unicode-range',
'units-per-em',
'v-alphabetic',
'v-hanging',
'v-ideographic',
'v-mathematical',
'widths',
'x-height',
]),
},
contentGroups: new Set(['descriptive']),
content: new Set([
// TODO: "at most one 'font-face-src' element"
Expand All @@ -1038,10 +1098,12 @@ export const elems = {
'font-face-format': {
attrsGroups: new Set(['core']),
attrs: new Set(['string']),
deprecated: { unsafe: new Set(['string']) },
},
'font-face-name': {
attrsGroups: new Set(['core']),
attrs: new Set(['name']),
deprecated: { unsafe: new Set(['name']) },
},
'font-face-src': {
attrsGroups: new Set(['core']),
Expand Down Expand Up @@ -1134,6 +1196,18 @@ export const elems = {
defaults: {
'arabic-form': 'initial',
},
deprecated: {
unsafe: new Set([
'arabic-form',
'glyph-name',
'horiz-adv-x',
'orientation',
'unicode',
'vert-adv-y',
'vert-origin-x',
'vert-origin-y',
]),
},
contentGroups: new Set([
'animation',
'descriptive',
Expand Down Expand Up @@ -1173,6 +1247,14 @@ export const elems = {
'vert-origin-x',
'vert-origin-y',
]),
deprecated: {
unsafe: new Set([
'horiz-adv-x',
'vert-adv-y',
'vert-origin-x',
'vert-origin-y',
]),
},
contentGroups: new Set([
'animation',
'descriptive',
Expand Down Expand Up @@ -1236,6 +1318,7 @@ export const elems = {
hkern: {
attrsGroups: new Set(['core']),
attrs: new Set(['u1', 'g1', 'u2', 'g2', 'k']),
deprecated: { unsafe: new Set(['g1', 'g2', 'k', 'u1', 'u2']) },
},
image: {
attrsGroups: new Set([
Expand Down Expand Up @@ -1430,6 +1513,14 @@ export const elems = {
'vert-origin-x',
'vert-origin-y',
]),
deprecated: {
unsafe: new Set([
'horiz-adv-x',
'vert-adv-y',
'vert-origin-x',
'vert-origin-y',
]),
},
contentGroups: new Set([
'animation',
'descriptive',
Expand Down Expand Up @@ -1710,6 +1801,15 @@ export const elems = {
contentScriptType: 'application/ecmascript',
contentStyleType: 'text/css',
},
deprecated: {
safe: new Set(['version']),
unsafe: new Set([
'baseProfile',
'contentScriptType',
'contentStyleType',
'zoomAndPan',
]),
},
contentGroups: new Set([
'animation',
'descriptive',
Expand Down Expand Up @@ -1956,11 +2056,13 @@ export const elems = {
'viewTarget',
'zoomAndPan',
]),
deprecated: { unsafe: new Set(['viewTarget', 'zoomAndPan']) },
contentGroups: new Set(['descriptive']),
},
vkern: {
attrsGroups: new Set(['core']),
attrs: new Set(['u1', 'g1', 'u2', 'g2', 'k']),
deprecated: { unsafe: new Set(['g1', 'g2', 'k', 'u1', 'u2']) },
},
};

Expand Down
3 changes: 3 additions & 0 deletions plugins/plugins-types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ type DefaultPlugins = {
removeComments: {
preservePatterns: Array<RegExp | string> | false;
};
removeDeprecatedAttrs: {
removeUnsafe?: boolean;
};
removeDesc: {
removeAny?: boolean;
};
Expand Down
2 changes: 2 additions & 0 deletions plugins/preset-default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createPreset } from '../lib/svgo/plugins.js';
import * as removeDoctype from './removeDoctype.js';
import * as removeXMLProcInst from './removeXMLProcInst.js';
import * as removeComments from './removeComments.js';
import * as removeDeprecatedAttrs from './removeDeprecatedAttrs.js';
import * as removeMetadata from './removeMetadata.js';
import * as removeEditorsNSData from './removeEditorsNSData.js';
import * as cleanupAttrs from './cleanupAttrs.js';
Expand Down Expand Up @@ -41,6 +42,7 @@ const presetDefault = createPreset({
removeDoctype,
removeXMLProcInst,
removeComments,
removeDeprecatedAttrs,
removeMetadata,
removeEditorsNSData,
cleanupAttrs,
Expand Down
120 changes: 120 additions & 0 deletions plugins/removeDeprecatedAttrs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import * as csswhat from 'css-what';
import { attrsGroupsDeprecated, elems } from './_collections.js';
import { collectStylesheet } from '../lib/style.js';

export const name = 'removeDeprecatedAttrs';
export const description = 'removes deprecated attributes';

/**
* @typedef {{ safe?: Set<string>; unsafe?: Set<string> }} DeprecatedAttrs
* @typedef {import('../lib/types.js').XastElement} XastElement
*/

/**
* @param {import('../lib/types.js').Stylesheet} stylesheet
* @returns {Set<string>}
*/
function extractAttributesInStylesheet(stylesheet) {
const attributesInStylesheet = new Set();

stylesheet.rules.forEach((rule) => {
const selectors = csswhat.parse(rule.selector);
selectors.forEach((subselector) => {
subselector.forEach((segment) => {
if (segment.type !== 'attribute') {
return;
}

attributesInStylesheet.add(segment.name);
});
});
});

return attributesInStylesheet;
}

/**
* @param {XastElement} node
* @param {DeprecatedAttrs | undefined} deprecatedAttrs
* @param {import('./plugins-types.js').DefaultPlugins['removeDeprecatedAttrs']} params
* @param {Set<string>} attributesInStylesheet
*/
function processAttributes(
node,
deprecatedAttrs,
params,
attributesInStylesheet,
) {
if (!deprecatedAttrs) {
return;
}

if (deprecatedAttrs.safe) {
deprecatedAttrs.safe.forEach((name) => {
if (attributesInStylesheet.has(name)) {
return;
}
delete node.attributes[name];
});
}

if (params.removeUnsafe && deprecatedAttrs.unsafe) {
deprecatedAttrs.unsafe.forEach((name) => {
if (attributesInStylesheet.has(name)) {
return;
}
delete node.attributes[name];
});
}
}

/**
* Remove deprecated attributes.
*
* @type {import('./plugins-types.js').Plugin<'removeDeprecatedAttrs'>}
*/
export function fn(root, params) {
const stylesheet = collectStylesheet(root);
const attributesInStylesheet = extractAttributesInStylesheet(stylesheet);

return {
element: {
enter: (node) => {
const elemConfig = elems[node.name];
if (!elemConfig) {
return;
}

// Special cases

// Removing deprecated xml:lang is safe when the lang attribute exists.
if (
elemConfig.attrsGroups.has('core') &&
node.attributes['xml:lang'] &&
!attributesInStylesheet.has('xml:lang') &&
node.attributes['lang']
) {
delete node.attributes['xml:lang'];
}

// General cases

elemConfig.attrsGroups.forEach((attrsGroup) => {
processAttributes(
node,
attrsGroupsDeprecated[attrsGroup],
params,
attributesInStylesheet,
);
});

processAttributes(
node,
elemConfig.deprecated,
params,
attributesInStylesheet,
);
},
},
};
}
Loading