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

Preserve whitespace in elements containing text #1220

Merged
merged 2 commits into from
Feb 23, 2021

Conversation

chromakode
Copy link
Contributor

Problem

SVGO configures its SAX parser to normalize whitespace, collapsing multiple space characters into one.

This can change the semantic meaning of SVG content:

Solution

This patch disables space normalization in sax. It removes trimming for node names classified as "textContent" in _collections.js. In addition, it changes the pretty printing logic to not add indents or whitespace when outputting such nodes.

I needed to update a few text fixtures which captured the previous space normalizing behavior. Existing plugin tests have a mixture of text nodes containing and not containing extra whitespace, so they cover the change in functionality. I've also added a test to svg2js to verify that the whitespace is preserved when parsing SVGs.

Related issues

src: local("Some Font"),
local("SomeFont"),
url(SomeFont.ttf);
src: local("Some Font"), local("SomeFont"), url(SomeFont.ttf);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

csso preserves the newlines in this src value, which were previously being turned into spaces by the sax normalization.

@felixfbecker
Copy link

@GreLI It would be really great to see this merged 🙇

@TrySound
Copy link
Member

Please rebase

@chromakode
Copy link
Contributor Author

@TrySound rebased 👍

// Save info about <text> tag to prevent trimming of meaningful whitespace
if (data.name == 'text' && !data.prefix) {
// Save info about tags containing text to prevent trimming of meaningful whitespace
if (textElems.indexOf(data.name) !== -1 && !data.prefix) {
Copy link
Member

Choose a reason for hiding this comment

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

Use includes array method please

@@ -4,12 +4,13 @@ var SAX = require('sax'),
JSAPI = require('./jsAPI.js'),
CSSClassList = require('./css-class-list'),
CSSStyleDeclaration = require('./css-style-declaration'),
entityDeclaration = /<!ENTITY\s+(\S+)\s+(?:'([^']+)'|"([^"]+)")\s*>/g;
entityDeclaration = /<!ENTITY\s+(\S+)\s+(?:'([^']+)'|"([^"]+)")\s*>/g,
textElems = require('../../plugins/_collections.js').textElems;
Copy link
Member

Choose a reason for hiding this comment

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

Put require before regexp

@TrySound TrySound merged commit 9f084cc into svg:master Feb 23, 2021
@TrySound
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants