From 999bf536c98e152a7151f0a69eacee73925adb41 Mon Sep 17 00:00:00 2001 From: Matan Date: Mon, 27 Jul 2026 08:42:54 +0300 Subject: [PATCH] fix: changing block type over a multi-block selection destroys text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting several blocks and toggling a heading (or Text) leaves every block holding the FIRST block's text. The other blocks' content is gone — not merely re-typed or re-ordered, but overwritten and unrecoverable through undo once the document is saved. The cause is one line, in three places: final delta = (node.delta ?? Delta()).toJson(); // read once: first block editorState.formatNode(selection, (node) => node.copyWith( attributes: {blockComponentDelta: delta}, // written to EVERY block )); `formatNode` runs its callback for every node in the selection, so hoisting the delta out of the callback stamps one block's text over all the others. The delta is now read from the node being transformed. Two details made this easier to hit than it looks: - `heading_toolbar_items.dart` and `paragraph_toolbar_item.dart` also captured `selection` at widget *build* time, so a press could apply to a selection that no longer existed. Both now read `editorState.selection` when pressed. This is why `onlyShowInSingleSelectionAndTextType` did not prevent it: the guard is evaluated at build time against the old selection. - `heading_command_shortcut.dart` has no single-selection gate at all, so the heading keyboard shortcuts could apply to a selection of any size. Reproduced by the added test in `test/new/block_component/heading_block_component/heading_command_shortcut_test.dart`, which fails on the current code with ['first', 'first', 'first'] and passes with the fix as ['first', 'second', 'third']. This was found after it destroyed a paragraph in a real document. --- .../heading_command_shortcut.dart | 9 ++-- .../desktop/items/heading_toolbar_items.dart | 46 ++++++++++++------- .../desktop/items/paragraph_toolbar_item.dart | 36 +++++++++------ .../heading_command_shortcut_test.dart | 40 ++++++++++++++++ 4 files changed, 97 insertions(+), 34 deletions(-) diff --git a/lib/src/editor/block_component/heading_block_component/heading_command_shortcut.dart b/lib/src/editor/block_component/heading_block_component/heading_command_shortcut.dart index 6bde34acd..6c985512c 100644 --- a/lib/src/editor/block_component/heading_block_component/heading_command_shortcut.dart +++ b/lib/src/editor/block_component/heading_block_component/heading_command_shortcut.dart @@ -74,8 +74,11 @@ KeyEventResult _toggleAttribute( node.type == HeadingBlockKeys.type && node.attributes[HeadingBlockKeys.level] == level; - final delta = (node.delta ?? Delta()).toJson(); - + // The delta is read per node inside the callback. Reading it once from + // `selection.start` and reusing it here wrote the first block's text over + // every block in a multi-block selection, destroying the rest -- and this + // shortcut path has no single-selection gate at all, so it was reachable + // with any selection. See heading_toolbar_items.dart (2026-07-27). editorState.formatNode( selection, (node) => node.copyWith( @@ -86,7 +89,7 @@ KeyEventResult _toggleAttribute( node.attributes[blockComponentBackgroundColor], blockComponentTextDirection: node.attributes[blockComponentTextDirection], - blockComponentDelta: delta, + blockComponentDelta: (node.delta ?? Delta()).toJson(), }, ), ); diff --git a/lib/src/editor/toolbar/desktop/items/heading_toolbar_items.dart b/lib/src/editor/toolbar/desktop/items/heading_toolbar_items.dart index ec1164797..a449db4e7 100644 --- a/lib/src/editor/toolbar/desktop/items/heading_toolbar_items.dart +++ b/lib/src/editor/toolbar/desktop/items/heading_toolbar_items.dart @@ -23,28 +23,40 @@ class _HeadingToolbarItem extends ToolbarItem { final node = editorState.getNodeAtPath(selection.start.path)!; final isHighlight = node.type == 'heading' && node.attributes['level'] == level; - final delta = (node.delta ?? Delta()).toJson(); final child = SVGIconItemWidget( iconName: 'toolbar/h$level', isHighlight: isHighlight, highlightColor: highlightColor, iconColor: iconColor, - onPressed: () => editorState.formatNode( - selection, - (node) => node.copyWith( - type: isHighlight - ? ParagraphBlockKeys.type - : HeadingBlockKeys.type, - attributes: { - HeadingBlockKeys.level: level, - blockComponentBackgroundColor: - node.attributes[blockComponentBackgroundColor], - blockComponentTextDirection: - node.attributes[blockComponentTextDirection], - blockComponentDelta: delta, - }, - ), - ), + onPressed: () { + // Both the selection and each node's delta are read HERE, not + // captured when this item was built. Hoisting them out was real + // data loss: `formatNode` runs the callback for every node in + // the selection, so one block's text was written over all of + // them and the others' writing was destroyed (2026-07-27). The + // build-time capture also went stale whenever the selection + // grew while the toolbar stayed on screen. + final selection = editorState.selection; + if (selection == null) { + return; + } + editorState.formatNode( + selection, + (node) => node.copyWith( + type: isHighlight + ? ParagraphBlockKeys.type + : HeadingBlockKeys.type, + attributes: { + HeadingBlockKeys.level: level, + blockComponentBackgroundColor: + node.attributes[blockComponentBackgroundColor], + blockComponentTextDirection: + node.attributes[blockComponentTextDirection], + blockComponentDelta: (node.delta ?? Delta()).toJson(), + }, + ), + ); + }, ); if (tooltipBuilder != null) { diff --git a/lib/src/editor/toolbar/desktop/items/paragraph_toolbar_item.dart b/lib/src/editor/toolbar/desktop/items/paragraph_toolbar_item.dart index b465fc19b..87b597266 100644 --- a/lib/src/editor/toolbar/desktop/items/paragraph_toolbar_item.dart +++ b/lib/src/editor/toolbar/desktop/items/paragraph_toolbar_item.dart @@ -10,25 +10,33 @@ final ToolbarItem paragraphItem = ToolbarItem( final selection = editorState.selection!; final node = editorState.getNodeAtPath(selection.start.path)!; final isHighlight = node.type == 'paragraph'; - final delta = (node.delta ?? Delta()).toJson(); final child = SVGIconItemWidget( iconName: 'toolbar/text', isHighlight: isHighlight, highlightColor: highlightColor, iconColor: iconColor, - onPressed: () => editorState.formatNode( - selection, - (node) => node.copyWith( - type: ParagraphBlockKeys.type, - attributes: { - blockComponentDelta: delta, - blockComponentBackgroundColor: - node.attributes[blockComponentBackgroundColor], - blockComponentTextDirection: - node.attributes[blockComponentTextDirection], - }, - ), - ), + // Read the selection and each node's delta at press time. See the note in + // heading_toolbar_items.dart -- hoisting the delta out of this callback + // wrote one block's text over every block in the selection. + onPressed: () { + final selection = editorState.selection; + if (selection == null) { + return; + } + editorState.formatNode( + selection, + (node) => node.copyWith( + type: ParagraphBlockKeys.type, + attributes: { + blockComponentDelta: (node.delta ?? Delta()).toJson(), + blockComponentBackgroundColor: + node.attributes[blockComponentBackgroundColor], + blockComponentTextDirection: + node.attributes[blockComponentTextDirection], + }, + ), + ); + }, ); if (tooltipBuilder != null) { diff --git a/test/new/block_component/heading_block_component/heading_command_shortcut_test.dart b/test/new/block_component/heading_block_component/heading_command_shortcut_test.dart index cb8006a65..443c83e9d 100644 --- a/test/new/block_component/heading_block_component/heading_command_shortcut_test.dart +++ b/test/new/block_component/heading_block_component/heading_command_shortcut_test.dart @@ -149,5 +149,45 @@ void main() async { node = editorState.getNodeAtPath([0])!; expect(node.type, ParagraphBlockKeys.type); }); + + // Regression test for real data loss on a real page, 2026-07-27. + // + // The delta was read ONCE from the block at `selection.start` and then + // written into every block the format callback ran for. Toggling a heading + // over a multi-block selection therefore stamped the first block's text + // over all the others and destroyed their writing. This shortcut path has + // no single-selection gate, so any selection could trigger it. + test('toggling a heading over several blocks keeps each blocks own text', + () { + final document = Document.blank() + ..addParagraph(initialText: 'first') + ..addParagraph(initialText: 'second') + ..addParagraph(initialText: 'third'); + final editorState = EditorState(document: document); + + editorState.selection = Selection( + start: Position(path: [0]), + end: Position(path: [2], offset: 'third'.length), + ); + toggleH1.execute(editorState); + + final texts = editorState.document.root.children + .map((n) => n.delta?.toPlainText()) + .toList(); + expect(texts, ['first', 'second', 'third']); + expect( + editorState.document.root.children.map((n) => n.type), + everyElement(HeadingBlockKeys.type), + ); + + // ...and back again, which is the same callback with the other type. + toggleH1.execute(editorState); + expect( + editorState.document.root.children + .map((n) => n.delta?.toPlainText()) + .toList(), + ['first', 'second', 'third'], + ); + }); }); }