Skip to content

Table drag: view.tablePos is never remapped through tr.mapping, throws on a concurrent edit mid-drag #2921

Description

@mustafa-yilmaz

Summary

While building a table-drag visual-feedback example (#2920), a reviewer asked about behavior when two users edit a table at the same time. Investigating turned up a crash bug in TableHandlesExtension itself, unrelated to that PR's own code.

Root cause

In packages/core/src/extensions/TableHandles/TableHandles.ts, TableHandlesView.tablePos is set once per mousemove:

this.tablePos = pmNodeInfo.posBeforeNode + 1;

It's then used directly, without ever being remapped through tr.mapping, in several places - notably the drop-cursor decorations():

const tableResolvedPos = state.doc.resolve(view.tablePos + 1);

mousemove doesn't fire on the dragged-over element while a native HTML5 drag is in progress, so tablePos (and the state.block content snapshot used later by dropHandler) can't refresh during a drag. If any transaction changes the document elsewhere while a drag is active - a concurrent local edit, or another collaborator's change over Yjs - tablePos goes stale.

Reproduction

  1. Start dragging a table row (mousedown on the row handle, then move over another row without releasing).
  2. While the drag is still in progress, dispatch any transaction that changes the document before the table (e.g. editor._tiptapEditor.view.dispatch(editor._tiptapEditor.view.state.tr.insertText("X", 1, 1)) to insert into a heading above the table).
  3. The next dragover recomputes the drop-cursor decoration from the now-stale tablePos, which resolves into the wrong node and throws:
RangeError: Index 1 out of range for <"...heading text...">
  at posAtIndex (prosemirror-model)
  at TableHandles.ts:695 (decorations)

This throws out of viewDecorationsupdateStateInnerdispatchTransaction, i.e. it breaks the transaction dispatch for the other user's edit, not just the drag.

Suggested fix

Same pattern already used correctly elsewhere for stored positions across transactions: remap tablePos (and any other stored position/snapshot used mid-drag) through tr.mapping in an appendTransaction/plugin apply, rather than only refreshing it on mousemove. Happy to open a PR for this if useful, but wanted to report it as its own issue since it's unrelated to the table-drag-visualization example in #2920 where it was found.

Environment

Found against main while working from a local build of packages/core/packages/react/packages/mantine.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions