fix: use wholeText for only contenteditable for set_data (#8394)

- split logic up into "is this a contenteditable element" and depending on the outcome use either .wholeText or .data to check if an update is necessary
- add to puppeteer because jsdom does not support contenteditable
- one test is skipped it because it fails right now but helps test #5018

---------

Co-authored-by: suxin2017 <[email protected]>
Co-authored-by: Simon H <[email protected]>
Co-authored-by: Simon Holthausen <[email protected]>

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -174,6 +174,8 @@ export default class ElementWrapper extends Wrapper {
 	child_dynamic_element_block?: Block = null;
 	child_dynamic_element?: ElementWrapper = null;
 
+	element_data_name = null;
+
 	constructor(
 		renderer: Renderer,
 		block: Block,
@@ -287,6 +289,8 @@ export default class ElementWrapper extends Wrapper {
 		}
 
 		this.fragment = new FragmentWrapper(renderer, block, node.children, this, strip_whitespace, next_sibling);
+
+		this.element_data_name = block.get_unique_name(`${this.var.name}_data`);
 	}
 
 	render(block: Block, parent_node: Identifier, parent_nodes: Identifier) {
@@ -516,7 +520,8 @@ export default class ElementWrapper extends Wrapper {
 				child.render(
 					block,
 					is_template ? x`${node}.content` : node,
-					nodes
+					nodes,
+					{ element_data_name: this.element_data_name }
 				);
 			});
 		}
@@ -824,7 +829,6 @@ export default class ElementWrapper extends Wrapper {
 
 	add_spread_attributes(block: Block) {
 		const levels = block.get_unique_name(`${this.var.name}_levels`);
-		const data = block.get_unique_name(`${this.var.name}_data`);
 
 		const initial_props = [];
 		const updates = [];
@@ -855,9 +859,9 @@ export default class ElementWrapper extends Wrapper {
 		block.chunks.init.push(b`
 			let ${levels} = [${initial_props}];
 
-			let ${data} = {};
+			let ${this.element_data_name} = {};
 			for (let #i = 0; #i < ${levels}.length; #i += 1) {
-				${data} = @assign(${data}, ${levels}[#i]);
+				${this.element_data_name} = @assign(${this.element_data_name}, ${levels}[#i]);
 			}
 		`);
 
@@ -869,12 +873,12 @@ export default class ElementWrapper extends Wrapper {
 					: x`@set_attributes`;
 
 		block.chunks.hydrate.push(
-			b`${fn}(${this.var}, ${data});`
+			b`${fn}(${this.var}, ${this.element_data_name});`
 		);
 
 		if (this.has_dynamic_attribute) {
 			block.chunks.update.push(b`
-				${fn}(${this.var}, ${data} = @get_spread_update(${levels}, [
+				${fn}(${this.var}, ${this.element_data_name} = @get_spread_update(${levels}, [
 					${updates}
 				]));
 			`);
@@ -890,23 +894,23 @@ export default class ElementWrapper extends Wrapper {
 			}
 
 			block.chunks.mount.push(b`
-				'value' in ${data} && (${data}.multiple ? @select_options : @select_option)(${this.var}, ${data}.value);
+				'value' in ${this.element_data_name} && (${this.element_data_name}.multiple ? @select_options : @select_option)(${this.var}, ${this.element_data_name}.value);
 			`);
 
 			block.chunks.update.push(b`
-				if (${block.renderer.dirty(Array.from(dependencies))} && 'value' in ${data}) (${data}.multiple ? @select_options : @select_option)(${this.var}, ${data}.value);
+				if (${block.renderer.dirty(Array.from(dependencies))} && 'value' in ${this.element_data_name}) (${this.element_data_name}.multiple ? @select_options : @select_option)(${this.var}, ${this.element_data_name}.value);
 			`);
 		} else if (this.node.name === 'input' && this.attributes.find(attr => attr.node.name === 'value')) {
 			const type = this.node.get_static_attribute_value('type');
 			if (type === null || type === '' || type === 'text' || type === 'email' || type === 'password') {
 				block.chunks.mount.push(b`
-					if ('value' in ${data}) {
-						${this.var}.value = ${data}.value;
+					if ('value' in ${this.element_data_name}) {
+						${this.var}.value = ${this.element_data_name}.value;
 					}
 				`);
 				block.chunks.update.push(b`
-					if ('value' in ${data}) {
-						${this.var}.value = ${data}.value;
+					if ('value' in ${this.element_data_name}) {
+						${this.var}.value = ${this.element_data_name}.value;
 					}
 				`);
 			}
@@ -1220,8 +1224,8 @@ export default class ElementWrapper extends Wrapper {
 				if (this.dynamic_style_dependencies.size > 0) {
 					maybe_create_style_changed_var();
 					// If all dependencies are same as the style attribute dependencies, then we can skip the dirty check
-					condition = 
-						all_deps.size === this.dynamic_style_dependencies.size 
+					condition =
+						all_deps.size === this.dynamic_style_dependencies.size
 							? style_changed_var
 							: x`${style_changed_var} || ${condition}`;
 				}
@@ -1232,7 +1236,6 @@ export default class ElementWrapper extends Wrapper {
 					}
 				`);
 			}
-
 		});
 	}
 

src/compiler/compile/render_dom/wrappers/Fragment.ts

@@ -4,7 +4,7 @@ import Body from './Body';
 import DebugTag from './DebugTag';
 import Document from './Document';
 import EachBlock from './EachBlock';
-import Element from './Element/index';
+import Element from './Element';
 import Head from './Head';
 import IfBlock from './IfBlock';
 import KeyBlock from './KeyBlock';

src/compiler/compile/render_dom/wrappers/MustacheTag.ts

@@ -5,7 +5,9 @@ import Wrapper from './shared/Wrapper';
 import MustacheTag from '../../nodes/MustacheTag';
 import RawMustacheTag from '../../nodes/RawMustacheTag';
 import { x } from 'code-red';
-import { Identifier } from 'estree';
+import { Identifier, Expression } from 'estree';
+import ElementWrapper from './Element';
+import AttributeWrapper from './Element/Attribute';
 
 export default class MustacheTagWrapper extends Tag {
 	var: Identifier = { type: 'Identifier', name: 't' };
@@ -14,10 +16,40 @@ export default class MustacheTagWrapper extends Tag {
 		super(renderer, block, parent, node);
 	}
 
-	render(block: Block, parent_node: Identifier, parent_nodes: Identifier) {
+	render(block: Block, parent_node: Identifier, parent_nodes: Identifier, data: Record<string, unknown> | undefined) {
+		const contenteditable_attributes =
+			this.parent instanceof ElementWrapper &&
+			this.parent.attributes.filter((a) => a.node.name === 'contenteditable');
+
+		const spread_attributes =
+			this.parent instanceof ElementWrapper &&
+			this.parent.attributes.filter((a) => a.node.is_spread);
+
+		let contenteditable_attr_value: Expression | true | undefined = undefined;
+		if (contenteditable_attributes.length > 0) {
+			const attribute = contenteditable_attributes[0] as AttributeWrapper;
+			if ([true, 'true', ''].includes(attribute.node.get_static_value())) {
+				contenteditable_attr_value = true;
+			} else {
+				contenteditable_attr_value = x`${attribute.get_value(block)}`;
+			}
+		}	else if (spread_attributes.length > 0 && data.element_data_name) {
+			contenteditable_attr_value = x`${data.element_data_name}['contenteditable']`;
+		}
+
 		const { init } = this.rename_this_method(
 			block,
-			value => x`@set_data(${this.var}, ${value})`
+			value => {
+				if (contenteditable_attr_value) {
+					if (contenteditable_attr_value === true) {
+						return x`@set_data_contenteditable(${this.var}, ${value})`;
+					} else {
+						return x`@set_data_maybe_contenteditable(${this.var}, ${value}, ${contenteditable_attr_value})`;
+					}
+				} else {
+					return x`@set_data(${this.var}, ${value})`;
+				}
+			}
 		);
 
 		block.add_element(

src/compiler/compile/render_dom/wrappers/shared/Wrapper.ts

@@ -85,7 +85,7 @@ export default class Wrapper {
 		);
 	}
 
-	render(_block: Block, _parent_node: Identifier, _parent_nodes: Identifier) {
+	render(_block: Block, _parent_node: Identifier, _parent_nodes: Identifier, _data: Record<string, any> = undefined) {
 		throw Error('Wrapper class is not renderable');
 	}
 }

src/runtime/internal/dev.ts

@@ -1,6 +1,7 @@
 import { custom_event, append, append_hydration, insert, insert_hydration, detach, listen, attr } from './dom';
 import { SvelteComponent } from './Component';
 import { is_void } from '../../shared/utils/names';
+import { contenteditable_truthy_values } from './utils';
 
 export function dispatch_dev<T=any>(type: string, detail?: T) {
 	document.dispatchEvent(custom_event(type, { version: '__VERSION__', ...detail }, { bubbles: true }));
@@ -83,12 +84,26 @@ export function dataset_dev(node: HTMLElement, property: string, value?: any) {
 	dispatch_dev('SvelteDOMSetDataset', { node, property, value });
 }
 
-export function set_data_dev(text, data) {
+export function set_data_dev(text: Text, data: unknown) {
 	data = '' + data;
-	if (text.wholeText === data) return;
+	if (text.data === data) return;
+	dispatch_dev('SvelteDOMSetData', { node: text, data });
+	text.data = (data as string);
+}
 
+export function set_data_contenteditable_dev(text: Text, data: unknown) {
+	data = '' + data;
+	if (text.wholeText === data) return;
 	dispatch_dev('SvelteDOMSetData', { node: text, data });
-	text.data = data;
+	text.data = (data as string);
+}
+
+export function set_data_maybe_contenteditable_dev(text: Text, data: unknown, attr_value: string) {
+	if (~contenteditable_truthy_values.indexOf(attr_value)) {
+		set_data_contenteditable_dev(text, data);
+	} else {
+		set_data_dev(text, data);
+	}
 }
 
 export function validate_each_argument(arg) {

src/runtime/internal/dom.ts

@@ -1,4 +1,4 @@
-import { has_prop } from './utils';
+import { contenteditable_truthy_values, has_prop } from './utils';
 
 // Track which nodes are claimed during hydration. Unclaimed nodes can then be removed from the DOM
 // at the end of hydration without touching the remaining nodes.
@@ -581,9 +581,24 @@ export function claim_html_tag(nodes, is_svg: boolean) {
 	return new HtmlTagHydration(claimed_nodes, is_svg);
 }
 
-export function set_data(text, data) {
+export function set_data(text: Text, data: unknown) {
 	data = '' + data;
-	if (text.wholeText !== data) text.data = data;
+	if (text.data === data) return;
+	text.data = (data as string);
+}
+
+export function set_data_contenteditable(text: Text, data: unknown) {
+	data = '' + data;
+	if (text.wholeText === data) return;
+	text.data = (data as string);
+}
+
+export function set_data_maybe_contenteditable(text: Text, data: unknown, attr_value: string) {
+	if (~contenteditable_truthy_values.indexOf(attr_value)) {
+		set_data_contenteditable(text, data);
+	} else {
+		set_data(text, data);
+	}
 }
 
 export function set_input_value(input, value) {

src/runtime/internal/utils.ts

@@ -194,3 +194,5 @@ export function split_css_unit(value: number | string): [number, string] {
 	const split = typeof value === 'string' && value.match(/^\s*(-?[\d.]+)([^\s]*)\s*$/);
 	return split ? [parseFloat(split[1]), split[2] || 'px'] : [value as number, 'px'];
 }
+
+export const contenteditable_truthy_values = ['', true, 1, 'true', 'contenteditable'];

test/runtime-puppeteer/samples/component-event-handler-contenteditable-false/_config.js

@@ -0,0 +1,13 @@
+// A puppeteer test because JSDOM doesn't support contenteditable
+export default {
+	html: '<div contenteditable="false"></div>',
+
+	async test({ assert, target, component, window }) {
+		const div = target.querySelector('div');
+		const text =  window.document.createTextNode('a');
+		div.insertBefore(text, null);
+		assert.equal(div.textContent, 'a');
+		component.text = 'bcde';
+		assert.equal(div.textContent, 'bcdea');
+	}
+};

test/runtime-puppeteer/samples/component-event-handler-contenteditable-false/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	export let text = '';
+	const updater = (event) => {text = event.target.textContent}
+</script>
+
+<div contenteditable="false" on:input={updater}>{text}</div>

test/runtime-puppeteer/samples/component-event-handler-contenteditable-spread/_config.js

@@ -0,0 +1,24 @@
+// A puppeteer test because JSDOM doesn't support contenteditable
+export default {
+	html: '<div contenteditable="true"></div>',
+	ssrHtml: '<div contenteditable=""></div>',
+
+	async test({ assert, target, window }) {
+		// this tests that by going from contenteditable=true to false, the
+		// content is correctly updated before that. This relies on the order
+		// of the updates: first updating the content, then setting contenteditable
+		// to false, which means that `set_data_maybe_contenteditable` is used and not `set_data`.
+		// If the order is reversed, https://github.com/sveltejs/svelte/issues/5018
+		// would be happening. The caveat is that if we go from contenteditable=false to true
+		// then we will have the same issue. To fix this reliably we probably need to
+		// overhaul the way we handle text updates in general.
+		// If due to some refactoring this test fails, it's probably fine to ignore it since
+		// this is a very specific edge case and the behavior is unstable anyway.
+		const div = target.querySelector('div');
+		const text =  window.document.createTextNode('a');
+		div.insertBefore(text, null);
+		const event = new window.InputEvent('input');
+		await div.dispatchEvent(event);
+		assert.equal(div.textContent, 'a');
+	}
+};

test/runtime-puppeteer/samples/component-event-handler-contenteditable-spread/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	let text = "";
+	const updater = (event) => {
+		text = event.target.textContent;
+	};
+	$: spread = {
+		contenteditable: text !== "a",
+	};
+</script>
+
+<div {...spread} on:input={updater}>{text}</div>

test/runtime-puppeteer/samples/component-event-handler-contenteditable/_config.js

@@ -0,0 +1,33 @@
+// A puppeteer test because JSDOM doesn't support contenteditable
+export default {
+	html: '<div contenteditable=""></div>',
+
+	// Failing test for https://github.com/sveltejs/svelte/issues/5018, fix pending
+	// It's hard to fix this because in order to do that, we would need to change the
+	// way the value is compared completely. Right now it compares the value of the
+	// first text node, but it should compare the value of the whole content
+	skip: true,
+
+	async test({ assert, target, window }) {
+		const div = target.querySelector('div');
+
+		let text = window.document.createTextNode('a');
+		div.insertBefore(text, null);
+		let event = new window.InputEvent('input');
+		await div.dispatchEvent(event);
+		assert.equal(div.textContent, 'a');
+
+		// When a user types a newline, the browser inserts a <div> element
+		const inner_div = window.document.createElement('div');
+		div.insertBefore(inner_div, null);
+		event = new window.InputEvent('input');
+		await div.dispatchEvent(event);
+		assert.equal(div.textContent, 'a');
+
+		text = window.document.createTextNode('b');
+		inner_div.insertBefore(text, null);
+		event = new window.InputEvent('input');
+		await div.dispatchEvent(event);
+		assert.equal(div.textContent, 'ab');
+	}
+};

test/runtime/samples/reactive-values-text-node/_config.js

@@ -0,0 +1,9 @@
+export default {
+	html:'<div>same text</div>',
+	async test({ assert, target }) {
+		await new Promise(f => setTimeout(f, 10));
+		assert.htmlEqual(target.innerHTML, `
+			<div>same text text</div>
+		`);
+	}
+};

test/runtime/samples/reactive-values-text-node/main.svelte

@@ -0,0 +1,8 @@
+<script>
+	let text = 'same';
+	setTimeout(() => {
+		text = 'same text';
+	}, 5);
+</script>
+
+<div>{text} text</div>