Skip to content

Commit

Permalink
[fix] strip leading newline after <pre> and <textarea> (#7280)
Browse files Browse the repository at this point in the history
Fixes #7264
  • Loading branch information
ota-meshi committed Apr 14, 2022
1 parent 9778eef commit 3a238fe
Show file tree
Hide file tree
Showing 22 changed files with 313 additions and 56 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,7 +6,8 @@
* Fix `{@const}` tag not working inside Component when there's no `let:` [#7189](https://github.com/sveltejs/svelte/issues/7189)
* Ignore comments in `{#each}` blocks when containing elements with `animate:` ([#3999](https://github.com/sveltejs/svelte/issues/3999))
* Add a third parameter to the returned function of `createEventDispatcher` that allows passing an object of `{ cancelable: true }` to create a cancelable custom event. The returned function when called will also return a boolean depending on whether the event is cancelled ([#7064](https://github.com/sveltejs/svelte/pull/7064))
* Fix value of `let:` bindings not updating in certain cases ([#7440](https://github.com/sveltejs/svelte/issues/7440))
* Fix value of `let:` bindings not updating in certain cases ([#7440](https://github.com/sveltejs/svelte/issues/7440))
* Strip leading newline after `<pre>` and `<textarea>` ([#7264](https://github.com/sveltejs/svelte/issues/7264))

## 3.47.0

Expand Down
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
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);
}
}

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
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

0 comments on commit 3a238fe

Please sign in to comment.