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

[fix]: strip leading newline after <pre> and <textarea> #7280

Merged
merged 13 commits into from Apr 14, 2022
16 changes: 15 additions & 1 deletion src/compiler/compile/nodes/Element.ts
Expand Up @@ -11,7 +11,7 @@ import StyleDirective from './StyleDirective';
import Text from './Text';
import { namespaces } from '../../utils/namespaces';
import map_children from './shared/map_children';
import { dimensions } from '../../utils/patterns';
import { dimensions, start_newline } from '../../utils/patterns';
import fuzzymatch from '../../utils/fuzzymatch';
import list from '../../utils/list';
import Let from './Let';
Expand Down Expand Up @@ -216,6 +216,20 @@ export default class Element extends Node {
this.namespace = get_namespace(parent as Element, this, component.namespace);

if (this.namespace !== namespaces.foreign) {
if (this.name === 'pre' || this.name === 'textarea') {
const first = info.children[0];
if (first && first.type === 'Text') {
// The leading newline character needs to be stripped because of a qirk,
// it is ignored by browsers if the tag and its contents are set through
// innerHTML (NOT if set through the innerHTML of the tag or dynamically).
// Therefore strip it here but add it back in the appropriate
// places if there's another newline afterwards.
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
// see https://html.spec.whatwg.org/multipage/grouping-content.html#the-pre-element
first.data = first.data.replace(start_newline, '');
}
}

if (this.name === 'textarea') {
if (info.children.length > 0) {
const value_attribute = info.attributes.find(node => node.name === 'value');
Expand Down
64 changes: 50 additions & 14 deletions src/compiler/compile/render_dom/wrappers/Element/index.ts
Expand Up @@ -12,7 +12,7 @@ import { namespaces } from '../../../../utils/namespaces';
import AttributeWrapper from './Attribute';
import StyleAttributeWrapper from './StyleAttribute';
import SpreadAttributeWrapper from './SpreadAttribute';
import { dimensions } from '../../../../utils/patterns';
import { dimensions, start_newline } from '../../../../utils/patterns';
import Binding from './Binding';
import add_to_set from '../../../utils/add_to_set';
import { add_event_handler } from '../shared/add_event_handlers';
Expand Down Expand Up @@ -1114,6 +1114,9 @@ export default class ElementWrapper extends Wrapper {
function to_html(wrappers: Array<ElementWrapper | TextWrapper | MustacheTagWrapper | RawMustacheTagWrapper>, block: Block, literal: any, state: any, can_use_raw_text?: boolean) {
wrappers.forEach(wrapper => {
if (wrapper instanceof TextWrapper) {
// Don't add the <pre>/<textare> newline logic here because pre/textarea.innerHTML
// would keep the leading newline, too, only someParent.innerHTML = '..<pre/textarea>..' won't

if ((wrapper as TextWrapper).use_space()) state.quasi.value.raw += ' ';

const parent = wrapper.node.parent as Element;
Expand Down Expand Up @@ -1141,29 +1144,46 @@ function to_html(wrappers: Array<ElementWrapper | TextWrapper | MustacheTagWrapp
// element
state.quasi.value.raw += `<${wrapper.node.name}`;

const is_empty_textarea = wrapper.node.name === 'textarea' && wrapper.fragment.nodes.length === 0;

(wrapper as ElementWrapper).attributes.forEach((attr: AttributeWrapper) => {
if (is_empty_textarea && attr.node.name === 'value') {
// The value attribute of <textarea> renders as content.
return;
}
state.quasi.value.raw += ` ${fix_attribute_casing(attr.node.name)}="`;

attr.node.chunks.forEach(chunk => {
if (chunk.type === 'Text') {
state.quasi.value.raw += escape_html(chunk.data);
} else {
literal.quasis.push(state.quasi);
literal.expressions.push(chunk.manipulate(block));

state.quasi = {
type: 'TemplateElement',
value: { raw: '' }
};
}
});
to_html_for_attr_value(attr, block, literal, state);

state.quasi.value.raw += '"';
});

if (!wrapper.void) {
state.quasi.value.raw += '>';

if (wrapper.node.name === 'pre') {
// Two or more leading newlines are required to restore the leading newline immediately after `<pre>`.
// see https://html.spec.whatwg.org/multipage/grouping-content.html#the-pre-element
tanhauhau marked this conversation as resolved.
Show resolved Hide resolved
const first = wrapper.fragment.nodes[0];
if (first && first.node.type === 'Text' && start_newline.test(first.node.data)) {
state.quasi.value.raw += '\n';
}
}

if (is_empty_textarea) {
// The <textarea> renders the value attribute as content because the content is stored in the value attribute.
const value_attribute = wrapper.attributes.find(attr => attr.node.name === 'value');
if (value_attribute) {
// Two or more leading newlines are required to restore the leading newline immediately after `<textarea>`.
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
const first = value_attribute.node.chunks[0];
if (first && first.type === 'Text' && start_newline.test(first.data)) {
state.quasi.value.raw += '\n';
}
to_html_for_attr_value(value_attribute, block, literal, state);
}
tanhauhau marked this conversation as resolved.
Show resolved Hide resolved
}

to_html(wrapper.fragment.nodes as Array<ElementWrapper | TextWrapper>, block, literal, state);

state.quasi.value.raw += `</${wrapper.node.name}>`;
Expand All @@ -1173,3 +1193,19 @@ function to_html(wrappers: Array<ElementWrapper | TextWrapper | MustacheTagWrapp
}
});
}

function to_html_for_attr_value(attr: AttributeWrapper | StyleAttributeWrapper | SpreadAttributeWrapper, block: Block, literal: any, state: any) {
attr.node.chunks.forEach(chunk => {
if (chunk.type === 'Text') {
state.quasi.value.raw += escape_html(chunk.data);
} else {
literal.quasis.push(state.quasi);
literal.expressions.push(chunk.manipulate(block));

state.quasi = {
type: 'TemplateElement',
value: { raw: '' }
};
}
});
}
23 changes: 22 additions & 1 deletion src/compiler/compile/render_ssr/handlers/Element.ts
Expand Up @@ -8,6 +8,7 @@ import Expression from '../../nodes/shared/Expression';
import remove_whitespace_children from './utils/remove_whitespace_children';
import fix_attribute_casing from '../../render_dom/wrappers/Element/fix_attribute_casing';
import { namespaces } from '../../../utils/namespaces';
import { start_newline } from '../../../utils/patterns';
import { Expression as ESExpression } from 'estree';

export default function (node: Element, renderer: Renderer, options: RenderOptions) {
Expand Down Expand Up @@ -42,7 +43,7 @@ export default function (node: Element, renderer: Renderer, options: RenderOptio
const { name, expression: { node: expression } } = style_directive;
return p`"${name}": ${expression}`;
});

const style_expression =
style_expression_list.length > 0 &&
x`{ ${style_expression_list} }`;
Expand Down Expand Up @@ -166,11 +167,31 @@ export default function (node: Element, renderer: Renderer, options: RenderOptio

renderer.add_expression(x`($$value => $$value === void 0 ? ${result} : $$value)(${node_contents})`);
} else {
if (node.name === 'textarea') {
// Two or more leading newlines are required to restore the leading newline immediately after `<textarea>`.
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
const value_attribute = node.attributes.find(({ name }) => name === 'value');
if (value_attribute) {
const first = value_attribute.chunks[0];
if (first && first.type === 'Text' && start_newline.test(first.data)) {
renderer.add_string('\n');
}
}
}
renderer.add_expression(node_contents);
}

add_close_tag();
} else {
if (node.name === 'pre') {
// Two or more leading newlines are required to restore the leading newline immediately after `<pre>`.
// see https://html.spec.whatwg.org/multipage/grouping-content.html#the-pre-element
// see https://html.spec.whatwg.org/multipage/syntax.html#element-restrictions
const first = children[0];
if (first && first.type === 'Text' && start_newline.test(first.data)) {
renderer.add_string('\n');
}
}
renderer.render(children, options);
add_close_tag();
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/utils/patterns.ts
@@ -1,5 +1,6 @@
export const whitespace = /[ \t\r\n]/;
export const start_whitespace = /^[ \t\r\n]*/;
export const end_whitespace = /[ \t\r\n]*$/;
export const start_newline = /^\r?\n/;

export const dimensions = /^(?:offset|client)(?:Width|Height)$/;
2 changes: 2 additions & 0 deletions test/runtime/samples/pre-tag/.editorconfig
@@ -0,0 +1,2 @@
[main.svelte]
trim_trailing_whitespace = unset
tanhauhau marked this conversation as resolved.
Show resolved Hide resolved
26 changes: 12 additions & 14 deletions test/runtime/samples/pre-tag/_config.js
Expand Up @@ -6,23 +6,14 @@ export default {
const elementDiv = target.querySelector('#div');
// Test for <pre> tag in non <pre> tag
const elementDivWithPre = target.querySelector('#div-with-pre');

// There is a slight difference in innerHTML because there is a difference in HTML optimization (in jsdom)
// depending on how the innerHTML is set.
// (There is no difference in the display.)
// Reassign innerHTML to add the same optimizations to innerHTML.

// eslint-disable-next-line no-self-assign
elementPre.innerHTML = elementPre.innerHTML;
// eslint-disable-next-line no-self-assign
elementDiv.innerHTML = elementDiv.innerHTML;
// eslint-disable-next-line no-self-assign
elementDivWithPre.innerHTML = elementDivWithPre.innerHTML;
// Test for <pre> tag with leading newline
const elementPreWithLeadingNewline = target.querySelector('#pre-with-leading-newline');
const elementPreWithoutLeadingNewline = target.querySelector('#pre-without-leading-newline');
const elementPreWithMultipleLeadingNewline = target.querySelector('#pre-with-multiple-leading-newlines');

assert.equal(
elementPre.innerHTML,
`
A
` A
B
<span>
C
Expand Down Expand Up @@ -53,5 +44,12 @@ export default {
F
</pre>`
);
assert.equal(elementPreWithLeadingNewline.children[0].innerHTML, 'leading newline');
assert.equal(elementPreWithLeadingNewline.children[1].innerHTML, ' leading newline and spaces');
assert.equal(elementPreWithLeadingNewline.children[2].innerHTML, '\nleading newlines');
assert.equal(elementPreWithoutLeadingNewline.children[0].innerHTML, 'without spaces');
assert.equal(elementPreWithoutLeadingNewline.children[1].innerHTML, ' with spaces ');
assert.equal(elementPreWithoutLeadingNewline.children[2].innerHTML, ' \nnewline after leading space');
assert.equal(elementPreWithMultipleLeadingNewline.innerHTML, '\n\nmultiple leading newlines');
}
};
22 changes: 22 additions & 0 deletions test/runtime/samples/pre-tag/main.svelte
Expand Up @@ -32,3 +32,25 @@
F
</pre>
</div>

<div id="pre-with-leading-newline">
<pre>
leading newline</pre>
<pre>
leading newline and spaces</pre>
<pre>

leading newlines</pre>
</div>

<div id="pre-without-leading-newline">
<pre>without spaces</pre>
<pre> with spaces </pre>
<pre>
newline after leading space</pre>
</div>

<pre id="pre-with-multiple-leading-newlines">


multiple leading newlines</pre>
15 changes: 1 addition & 14 deletions test/runtime/samples/preserve-whitespaces/_config.js
Expand Up @@ -10,22 +10,9 @@ export default {
// Test for <pre> tag in non <pre> tag
const elementDivWithPre = target.querySelector('#div-with-pre');

// There is a slight difference in innerHTML because there is a difference in HTML optimization (in jsdom)
// depending on how the innerHTML is set.
// (There is no difference in the display.)
// Reassign innerHTML to add the same optimizations to innerHTML.

// eslint-disable-next-line no-self-assign
elementPre.innerHTML = elementPre.innerHTML;
// eslint-disable-next-line no-self-assign
elementDiv.innerHTML = elementDiv.innerHTML;
// eslint-disable-next-line no-self-assign
elementDivWithPre.innerHTML = elementDivWithPre.innerHTML;

assert.equal(
elementPre.innerHTML,
`
A
` A
B
<span>
C
Expand Down
4 changes: 2 additions & 2 deletions test/runtime/samples/textarea-children/_config.js
Expand Up @@ -9,9 +9,9 @@ export default {

test({ assert, component, target }) {
const textarea = target.querySelector( 'textarea' );
assert.strictEqual( textarea.value, '\n\t<p>not actually an element. 42</p>\n' );
assert.strictEqual( textarea.value, '\t<p>not actually an element. 42</p>\n' );

component.foo = 43;
assert.strictEqual( textarea.value, '\n\t<p>not actually an element. 43</p>\n' );
assert.strictEqual( textarea.value, '\t<p>not actually an element. 43</p>\n' );
}
};
2 changes: 2 additions & 0 deletions test/runtime/samples/textarea-content/.editorconfig
@@ -0,0 +1,2 @@
[main.svelte]
trim_trailing_whitespace = unset
34 changes: 34 additions & 0 deletions test/runtime/samples/textarea-content/_config.js
@@ -0,0 +1,34 @@
export default {
test({ assert, target }) {
// Test for <textarea> tag
const elementTextarea = target.querySelector('#textarea');
// Test for <textarea> tag in non <textarea> tag
const elementDivWithTextarea = target.querySelector('#div-with-textarea');
// Test for <textarea> tag with leading newline
const elementTextareaWithLeadingNewline = target.querySelector('#textarea-with-leading-newline');
const elementTextareaWithoutLeadingNewline = target.querySelector('#textarea-without-leading-newline');
const elementTextareaWithMultipleLeadingNewline = target.querySelector('#textarea-with-multiple-leading-newlines');
const elementDivWithTextareaWithMultipleLeadingNewline = target.querySelector('#div-with-textarea-with-multiple-leading-newlines');

assert.equal(
elementTextarea.value,
` A
B
`
);
assert.equal(
elementDivWithTextarea.children[0].value,
` A
B
`
);
assert.equal(elementTextareaWithLeadingNewline.children[0].value, 'leading newline');
assert.equal(elementTextareaWithLeadingNewline.children[1].value, ' leading newline and spaces');
assert.equal(elementTextareaWithLeadingNewline.children[2].value, '\nleading newlines');
assert.equal(elementTextareaWithoutLeadingNewline.children[0].value, 'without spaces');
assert.equal(elementTextareaWithoutLeadingNewline.children[1].value, ' with spaces ');
assert.equal(elementTextareaWithoutLeadingNewline.children[2].value, ' \nnewline after leading space');
assert.equal(elementTextareaWithMultipleLeadingNewline.value, '\n\nmultiple leading newlines');
assert.equal(elementDivWithTextareaWithMultipleLeadingNewline.children[0].value, '\n\nmultiple leading newlines');
}
};
40 changes: 40 additions & 0 deletions test/runtime/samples/textarea-content/main.svelte
@@ -0,0 +1,40 @@
<textarea id="textarea">
A
B
</textarea>

<div id="div-with-textarea">
<textarea>
A
B
</textarea>
</div>

<div id="textarea-with-leading-newline">
<textarea>
leading newline</textarea>
<textarea>
leading newline and spaces</textarea>
<textarea>

leading newlines</textarea>
</div>

<div id="textarea-without-leading-newline">
<textarea>without spaces</textarea>
<textarea> with spaces </textarea>
<textarea>
newline after leading space</textarea>
</div>

<textarea id="textarea-with-multiple-leading-newlines">


multiple leading newlines</textarea>

<div id="div-with-textarea-with-multiple-leading-newlines">
<textarea>


multiple leading newlines</textarea>
</div>
2 changes: 1 addition & 1 deletion test/server-side-rendering/index.ts
Expand Up @@ -84,7 +84,7 @@ describe('ssr', () => {

try {
if (config.withoutNormalizeHtml) {
assert.strictEqual(html.trim(), expectedHtml.trim().replace(/\r\n/g, '\n'));
assert.strictEqual(html.trim().replace(/\r\n/g, '\n'), expectedHtml.trim().replace(/\r\n/g, '\n'));
} else {
(compileOptions.preserveComments
? assert.htmlEqualWithComments
Expand Down
2 changes: 2 additions & 0 deletions test/server-side-rendering/samples/pre-tag/.editorconfig
@@ -0,0 +1,2 @@
[{main.svelte,_expected.html}]
trim_trailing_whitespace = unset
tanhauhau marked this conversation as resolved.
Show resolved Hide resolved