svgo

svgo copied to clipboard

This PR adds the chainFilters plugin which chains filter elements using CSS filters as workaround for some issues with specific browsers, closes https://github.com/svg/svgo/issues/732. Furthermore a DOM walker is added which can be used for traversing the svgo DOM (by full mode plugins). Currently some plugins and svgo core all use their own custom "monkeys" implementations for this.

If a filter is already assigned to an element using styles and the plugin also got a filter attribute to the same filter element, this plugin would still create a chain - this may be a feature to add - but this kind of corner case should be very rare, also the element shouldn't have a filter assigned by filter attribute and styles in the first place. Also this plugin currently always assumes relative paths in fragment identifiers.

@drschwabe: You can already test this plugin using:

# using npm:
$ npm install github:strarsis/svgo#chain-filters

# using yarn
$ yarn add github:strarsis/svgo#chain-filters

@GreLI: The filter bug is indeed a problem in Firefox - maybe this plugin should be enabled by default? Currently the filter element is chained with the element using a CSS class with filter property via IRI. Would it be better to convert the whole filter element into CSS? - Would this be always possible?

@GreLI: What can I do for further speeding up the review/merge process?

I haven't made a decision yet. Not sure about the plugin. Need some time to consider.

@GreLI, @caub: Arguments against merging this plugin into svgo:

• It doesn't minify or simplify, rather it is a polyfill for a Firefox issue.
• (When a plugin-as-package architecture has been implemented in svgo) This could also fit in a separate package.
• I am biased because I made the plugin.

Arguments for merging this plugin into svgo:

• This Firefox bug can be subtle and puzzling when suddenly some parts of the SVG are missing when viewed in Firefox. Just having this extra step will ensure this problem doesn't happen again for all SVGs processed by svgo.
• One doesn't necessarily finds this bug when searching for it (I encountered this bug in a svgo issue created by another user and found the exact same symptoms).
• This plugin can always be disabled or even removed when Firefox has this bug finally fixed.

@TrySound: Does it make sense to add this plugin to svgo? Loading plugins from other packages (installed from npm) isn't supported yet by svgo.

@strarsis It is supported. Just require the plugin and pass to plugins array.

@TrySound: So should this plugin now be merged into svgo repository or rather provided as standalone plugin (e.g. via npm)?

This looks more like polyfill than optimisation. So yes, this would probably look better as separate package.

See

@TrySound: Thanks for the adjustment. I had not looked at this PR for some while.

When I try this test SVG in Chrome and Firefox (both, regular and Developer Edition), the SVG is displayed correctly and equally: https://gist.githubusercontent.com/strarsis/2c769b63b9573b9cf81faa9242adf4ec/raw/1571560b8ca1c895a52fb3669a4ad32615350993/chainfilters-test.svg So does this mean that this issue had been eventually fixed in Firefox?

Hey! Thanks for creating the PR.

So does this mean that this issue had been eventually fixed in Firefox?

Indeed, it looks like the issue has already been resolved. Makes sense given the amount of time that's passed. ^-^'