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 all white space within foreignObject elements. #2064

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 10 additions & 3 deletions lib/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const parseSvg = (data, from) => {
* @type {XastParent[]}
*/
const stack = [root];
let foreignLevel = 0;

/**
* @type {(node: XastChild) => void}
Expand Down Expand Up @@ -157,7 +158,7 @@ export const parseSvg = (data, from) => {
*/
const node = {
type: 'comment',
value: comment.trim(),
value: foreignLevel > 0 ? comment : comment.trim(),
};
pushToContent(node);
};
Expand Down Expand Up @@ -189,12 +190,15 @@ export const parseSvg = (data, from) => {
pushToContent(element);
current = element;
stack.push(element);
if (data.name === 'foreignObject') {
foreignLevel++;
}
};

sax.ontext = (text) => {
if (current.type === 'element') {
// prevent trimming of meaningful whitespace inside textual tags
if (textElems.has(current.name)) {
if (foreignLevel > 0 || textElems.has(current.name)) {
/**
* @type {XastText}
*/
Expand All @@ -216,9 +220,12 @@ export const parseSvg = (data, from) => {
}
};

sax.onclosetag = () => {
sax.onclosetag = (tagName) => {
stack.pop();
current = stack[stack.length - 1];
if (tagName === 'foreignObject') {
foreignLevel--;
}
};

sax.onerror = (e) => {
Expand Down
94 changes: 58 additions & 36 deletions lib/stringifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { textElems } from '../plugins/_collections.js';
* indent: string,
* textContext: ?XastElement,
* indentLevel: number,
* foreignLevel: number,
* eolLen: number
* }} State
* @typedef {Required<StringifyOptions>} Options
*/
Expand Down Expand Up @@ -81,14 +83,6 @@ export const stringifySvg = (data, userOptions = {}) => {
} else if (typeof indent === 'string') {
newIndent = indent;
}
/**
* @type {State}
*/
const state = {
indent: newIndent,
textContext: null,
indentLevel: 0,
};
const eol = config.eol === 'crlf' ? '\r\n' : '\n';
if (config.pretty) {
config.doctypeEnd += eol;
Expand All @@ -100,6 +94,16 @@ export const stringifySvg = (data, userOptions = {}) => {
config.tagCloseEnd += eol;
config.textEnd += eol;
}
/**
* @type {State}
*/
const state = {
indent: newIndent,
textContext: null,
indentLevel: 0,
foreignLevel: 0,
eolLen: eol.length,
};
let svg = stringifyNode(data, config, state);
if (config.finalNewline && svg.length > 0 && !svg.endsWith('\n')) {
svg += eol;
Expand All @@ -116,20 +120,15 @@ const stringifyNode = (data, config, state) => {
for (const item of data.children) {
if (item.type === 'element') {
svg += stringifyElement(item, config, state);
}
if (item.type === 'text') {
} else if (item.type === 'text') {
svg += stringifyText(item, config, state);
}
if (item.type === 'doctype') {
} else if (item.type === 'doctype') {
svg += stringifyDoctype(item, config);
}
if (item.type === 'instruction') {
} else if (item.type === 'instruction') {
svg += stringifyInstruction(item, config);
}
if (item.type === 'comment') {
svg += stringifyComment(item, config);
}
if (item.type === 'cdata') {
} else if (item.type === 'comment') {
svg += stringifyComment(item, config, state);
} else if (item.type === 'cdata') {
svg += stringifyCdata(item, config, state);
}
}
Expand All @@ -144,12 +143,25 @@ const stringifyNode = (data, config, state) => {
*/
const createIndent = (config, state) => {
let indent = '';
if (config.pretty && state.textContext == null) {
if (config.pretty && state.textContext == null && state.foreignLevel === 0) {
indent = state.indent.repeat(state.indentLevel - 1);
}
return indent;
};

/**
* Trim newline at end of tag if format is "pretty" and in a foreignObject.
* @param {string} tagEnd
* @param {StringifyOptions} config
* @param {State} state
*/
const formatEndTag = (tagEnd, config, state) => {
if (config.pretty && state.foreignLevel > 0) {
return tagEnd.substring(0, tagEnd.length - state.eolLen);
}
return tagEnd;
};

/**
* @type {(node: XastDoctype, config: Options) => string}
*/
Expand All @@ -167,10 +179,14 @@ const stringifyInstruction = (node, config) => {
};

/**
* @type {(node: XastComment, config: Options) => string}
* @type {(node: XastComment, config: Options,state:State) => string}
*/
const stringifyComment = (node, config) => {
return config.commentStart + node.value + config.commentEnd;
const stringifyComment = (node, config, state) => {
return (
config.commentStart +
node.value +
formatEndTag(config.commentEnd, config, state)
);
};

/**
Expand All @@ -181,7 +197,7 @@ const stringifyCdata = (node, config, state) => {
createIndent(config, state) +
config.cdataStart +
node.value +
config.cdataEnd
formatEndTag(config.cdataEnd, config, state)
);
};

Expand All @@ -197,18 +213,18 @@ const stringifyElement = (node, config, state) => {
config.tagShortStart +
node.name +
stringifyAttributes(node, config) +
config.tagShortEnd
formatEndTag(config.tagShortEnd, config, state)
);
} else {
return (
createIndent(config, state) +
config.tagShortStart +
node.name +
stringifyAttributes(node, config) +
config.tagOpenEnd +
formatEndTag(config.tagOpenEnd, config, state) +
config.tagCloseStart +
node.name +
config.tagCloseEnd
formatEndTag(config.tagCloseEnd, config, state)
);
}
// non-empty element
Expand All @@ -218,7 +234,7 @@ const stringifyElement = (node, config, state) => {
let tagCloseStart = config.tagCloseStart;
let tagCloseEnd = config.tagCloseEnd;
let openIndent = createIndent(config, state);
let closeIndent = createIndent(config, state);
let enableCloseIndent = true;

if (state.textContext) {
tagOpenStart = defaults.tagOpenStart;
Expand All @@ -229,28 +245,34 @@ const stringifyElement = (node, config, state) => {
} else if (textElems.has(node.name)) {
tagOpenEnd = defaults.tagOpenEnd;
tagCloseStart = defaults.tagCloseStart;
closeIndent = '';
enableCloseIndent = false;
state.textContext = node;
}

if (node.name === 'foreignObject') {
state.foreignLevel++;
}
const children = stringifyNode(node, config, state);

if (state.textContext === node) {
state.textContext = null;
}

return (
const s =
openIndent +
tagOpenStart +
node.name +
stringifyAttributes(node, config) +
tagOpenEnd +
formatEndTag(tagOpenEnd, config, state) +
children +
closeIndent +
(enableCloseIndent ? createIndent(config, state) : '') +
tagCloseStart +
node.name +
tagCloseEnd
);
node.name;
if (node.name === 'foreignObject') {
state.foreignLevel--;
}

return s + formatEndTag(tagCloseEnd, config, state);
}
};

Expand Down Expand Up @@ -281,6 +303,6 @@ const stringifyText = (node, config, state) => {
createIndent(config, state) +
config.textStart +
node.value.replace(config.regEntities, config.encodeEntity) +
(state.textContext ? '' : config.textEnd)
(state.textContext ? '' : formatEndTag(config.textEnd, config, state))
);
};
4 changes: 1 addition & 3 deletions plugins/_collections.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,8 @@ export const elemsGroups = {
/**
* Elements where adding or removing whitespace may effect rendering, metadata,
* or semantic meaning.
*
* @see https://developer.mozilla.org/docs/Web/HTML/Element/pre
*/
export const textElems = new Set([...elemsGroups.textContent, 'pre', 'title']);
export const textElems = new Set([...elemsGroups.textContent, 'title']);

export const pathElems = new Set(['glyph', 'missing-glyph', 'path']);

Expand Down
12 changes: 3 additions & 9 deletions test/plugins/inlineStyles.17.svg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@

<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
<foreignObject width="100%" height="100%">
<style>
div { color: red; }
</style>
<body xmlns="http://www.w3.org/1999/xhtml">
<div>
hello, world
</div>
</body>
</foreignObject>
<style>div { color: red; }</style>
<body xmlns="http://www.w3.org/1999/xhtml"><div>hello, world</div></body>
</foreignObject>
</svg>
8 changes: 4 additions & 4 deletions test/plugins/mergeStyles.12.svg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ Skip styles inside foreignObject element

<svg id="test" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100">
<foreignObject>
<style>
.st0 { fill: yellow; }
</style>
</foreignObject>
<style>
.st0 { fill: yellow; }
</style>
</foreignObject>
<style>
.st1 { fill: red; }
</style>
Expand Down
13 changes: 13 additions & 0 deletions test/svgo/_index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,17 @@ describe('svgo', () => {
});
expect(normalize(result.data)).toStrictEqual(expected);
});

it('should preserve comments and cdata in foreign object', async () => {
const [original, expected] = await parseFixture(
'foreign-comments-and-cdata-pretty.svg.txt',
);
// Disable plugins so comments aren't removed.
const result = optimize(original, {
path: 'input.svg',
plugins: [],
js2svg: { pretty: true },
});
expect(normalize(result.data)).toStrictEqual(expected);
});
});
27 changes: 27 additions & 0 deletions test/svgo/foreign-comments-and-cdata-pretty.svg.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<svg xmlns="http://www.w3.org/2000/svg">
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<!-- comment ... -->
Some random text

<![CDATA[
some more text
]]>
</div>
</foreignObject>
</svg>

@@@

<svg xmlns="http://www.w3.org/2000/svg">
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<!-- comment ... -->
Some random text

<![CDATA[
some more text
]]>
</div>
</foreignObject>
</svg>
8 changes: 4 additions & 4 deletions test/svgo/pre-element-pretty.svg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ M M A A IIIII N N T A A IIIII N N EEEEE R R </pre>

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 854 340">
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<pre style="text-align:center"> OOO PPPP EEEEE N N SSSSS OOO U U RRRR CCCC EEEEE
<div xmlns="http://www.w3.org/1999/xhtml">
<pre style="text-align:center"> OOO PPPP EEEEE N N SSSSS OOO U U RRRR CCCC EEEEE
O O P P E NN N SS O O U U R R C E
O O PPPP EEE N N N SSS O O U U RRRR C EEE
O O P E N NN SS O O U U R R C E
Expand All @@ -32,6 +32,6 @@ MM MM A A I NN N T A A I NN N E R R
M M M AAAAA I N N N T AAAAA I N N N EEE RRRR
M M A A I N NN T A A I N NN E R R
M M A A IIIII N N T A A IIIII N N EEEEE R R </pre>
</div>
</foreignObject>
</div>
</foreignObject>
</svg>
9 changes: 7 additions & 2 deletions test/svgo/pre-element.svg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ M M A A IIIII N N T A A IIIII N N EEEEE R R </pre>

@@@

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 854 340"><foreignObject width="100%" height="100%"><div xmlns="http://www.w3.org/1999/xhtml"><pre style="text-align:center"> OOO PPPP EEEEE N N SSSSS OOO U U RRRR CCCC EEEEE
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 854 340"><foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<pre style="text-align:center"> OOO PPPP EEEEE N N SSSSS OOO U U RRRR CCCC EEEEE
O O P P E NN N SS O O U U R R C E
O O PPPP EEE N N N SSS O O U U RRRR C EEE
O O P E N NN SS O O U U R R C E
Expand All @@ -28,4 +30,7 @@ M M AAA IIIII N N TTTTT AAA IIIII N N EEEEE RRRR
MM MM A A I NN N T A A I NN N E R R
M M M AAAAA I N N N T AAAAA I N N N EEE RRRR
M M A A I N NN T A A I N NN E R R
M M A A IIIII N N T A A IIIII N N EEEEE R R </pre></div></foreignObject></svg>
M M A A IIIII N N T A A IIIII N N EEEEE R R </pre>
</div>
</foreignObject></svg>