Skip to content

fix: changing block type over a multi-block selection destroys text - #1227

Open
matanrotman wants to merge 1 commit into
AppFlowy-IO:mainfrom
matanrotman:fix/multi-block-format-preserves-each-blocks-text
Open

fix: changing block type over a multi-block selection destroys text#1227
matanrotman wants to merge 1 commit into
AppFlowy-IO:mainfrom
matanrotman:fix/multi-block-format-preserves-each-blocks-text

Conversation

@matanrotman

Copy link
Copy Markdown

What's wrong

Selecting several blocks and toggling a heading — or switching them to Text — leaves every block holding the first block's text. The other blocks' content is overwritten and gone.

Cause

One line, in three files:

final delta = (node.delta ?? Delta()).toJson();   // read once: the first block
editorState.formatNode(selection, (node) => node.copyWith(
  attributes: {blockComponentDelta: delta},       // written into 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. This PR reads the delta from the node actually being transformed.

Two details that make it 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 doesn't 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 apply to a selection of any size.

Test

Added to test/new/block_component/heading_block_component/heading_command_shortcut_test.dart. On current main it reports:

Expected: ['first', 'second', 'third']
  Actual: ['first', 'first', 'first']

and passes with the fix.

Verification, honestly

The failing-then-passing run was done against a checkout whose heading_command_shortcut.dart is byte-identical to this branch's base, on Flutter 3.27.4. I could not compile this branch locally — appflowy_editor on main requires Flutter >=3.32.0 — so CI here is the first real compile. dart format is clean on all four files.

Context

Found after it destroyed a paragraph in a real document. The same mistake exists twice more in AppFlowy-IO/AppFlowy (text_heading_toolbar_item.dart, heading_toolbar_item.dart); a companion PR covers those.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant