Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,33 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
}
}

// When pasting into a textblock, try the open slice first so content merges inline
// instead of creating new paragraphs (prevents inserting block nodes into non-textblocks).
const baseParentIsTextblock = trTemp.doc.resolve(positionTo).parent?.isTextblock;
const shouldPreferInlineInsertion = step.from === step.to && baseParentIsTextblock;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the actual fix for the bug but there's no comment explaining what it does.

something like "when pasting into a textblock, try the open slice first so content merges inline instead of creating new paragraphs" would go a long way for anyone reading this later.


const tryInsert = (slice) => {
Comment thread
palmer-cl marked this conversation as resolved.
const insertionStep = new ReplaceStep(positionTo, positionTo, slice, false);
if (trTemp.maybeStep(insertionStep).failed) return null;
return {
insertedFrom: insertionStep.from,
insertedTo: insertionStep.getMap().map(insertionStep.to, 1),
};
const tempTr = state.apply(newTr).tr;
// Empty slices represent pure deletions (no content to insert).
// Detecting them ensures deletion tracking runs even if `tempTr` doesn't change.
const isEmptySlice = slice?.content?.size === 0;
Comment thread
palmer-cl marked this conversation as resolved.
try {
tempTr.replaceRange(positionTo, positionTo, slice);
} catch {
return null;
}

if (!tempTr.docChanged && !isEmptySlice) return null;

const insertedFrom = tempTr.mapping.map(positionTo, -1);
const insertedTo = tempTr.mapping.map(positionTo, 1);
if (insertedFrom === insertedTo) return { tempTr, insertedFrom, insertedTo };
if (shouldPreferInlineInsertion && !tempTr.doc.resolve(insertedFrom).parent?.isTextblock) return null;
return { tempTr, insertedFrom, insertedTo };
};

const insertion = tryInsert(step.slice) || tryInsert(Slice.maxOpen(step.slice.content, true));
const openSlice = Slice.maxOpen(step.slice.content, true);
const insertion = tryInsert(step.slice) || tryInsert(openSlice);

// If we can't insert the replacement content into the temp transaction, fall back to applying the original step.
// This keeps user intent (content change) even if we can't represent it as tracked insert+delete.
Expand All @@ -61,16 +78,22 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
}

const meta = {};
const insertedMark = markInsertion({
tr: trTemp,
from: insertion.insertedFrom,
to: insertion.insertedTo,
user,
date,
});
const { insertedFrom, insertedTo, tempTr } = insertion;
let insertedMark = null;
let trackedInsertedSlice = Slice.empty;

if (insertedFrom !== insertedTo) {
insertedMark = markInsertion({
tr: tempTr,
from: insertedFrom,
to: insertedTo,
user,
date,
});
trackedInsertedSlice = tempTr.doc.slice(insertedFrom, insertedTo);
}

// Condense insertion down to a single replace step (so this tracked transaction remains a single-step insertion).
const trackedInsertedSlice = trTemp.doc.slice(insertion.insertedFrom, insertion.insertedTo);
const condensedStep = new ReplaceStep(positionTo, positionTo, trackedInsertedSlice, false);
if (newTr.maybeStep(condensedStep).failed) {
// If the condensed step can't be applied, fall back to the original step and skip deletion tracking.
Expand All @@ -86,21 +109,21 @@ export const replaceStep = ({ state, tr, step, newTr, map, user, date, originalS
const mirrorIndex = map.maps.length - 1;
map.appendMap(condensedStep.getMap(), mirrorIndex);

if (insertion.insertedFrom !== insertion.insertedTo) {
if (insertedFrom !== insertedTo) {
meta.insertedMark = insertedMark;
meta.step = condensedStep;
// Store the actual insertion end position for cursor placement (SD-1624).
// Only needed when position was adjusted to insert after a deletion span.
// For single-step transactions, positionTo is in newTr.doc coordinates after our condensedStep,
// so we just add the insertion length to get the cursor position.
if (positionAdjusted) {
const insertionLength = insertion.insertedTo - insertion.insertedFrom;
const insertionLength = insertedTo - insertedFrom;
meta.insertedTo = positionTo + insertionLength;
}
}

if (!newTr.selection.eq(trTemp.selection)) {
newTr.setSelection(trTemp.selection);
if (!newTr.selection.eq(tempTr.selection)) {
newTr.setSelection(tempTr.selection);
}

if (step.from !== step.to) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the three new tests all paste at a cursor position (no selection).

could we add one where you select some text and paste over it? that would cover the other branch where shouldPreferInlineInsertion is false and the try order flips.

import { EditorState, TextSelection } from 'prosemirror-state';
import { DOMParser as PMDOMParser, Slice } from 'prosemirror-model';
import { trackedTransaction, documentHelpers } from './index.js';
import { TrackInsertMarkName, TrackDeleteMarkName } from '../constants.js';
import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js';
Expand Down Expand Up @@ -207,6 +208,169 @@ describe('trackChangesHelpers replaceStep', () => {
expect(insertedText).toBe('xy');
});

it('tracks single-paragraph HTML paste insertions', () => {
const doc = schema.nodes.doc.create(
{},
schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Base')])),
);
let state = createState(doc);

const basePos = findTextPos(state.doc, 'Base');
expect(basePos).toBeTypeOf('number');
const insertPos = basePos + 'Base'.length;
state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, insertPos)));

const tempDiv = document.createElement('div');
tempDiv.innerHTML = '<p>Paste One</p>';
const parsedDoc = PMDOMParser.fromSchema(schema).parse(tempDiv);
const slice = new Slice(parsedDoc.content, 0, 0);

let tr = state.tr.replaceSelection(slice);
tr.setMeta('inputType', 'insertFromPaste');
const tracked = trackedTransaction({ tr, state, user });
const finalState = state.apply(tracked);

let insertedText = '';
finalState.doc.descendants((node) => {
if (node.isText && node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) {
insertedText += node.text;
}
});

expect(insertedText).toContain('Paste One');
});

it('tracks multi-paragraph HTML paste insertions', () => {
const doc = schema.nodes.doc.create(
{},
schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Base')])),
);
let state = createState(doc);

const basePos = findTextPos(state.doc, 'Base');
expect(basePos).toBeTypeOf('number');
const insertPos = basePos + 'Base'.length;
state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, insertPos)));

const tempDiv = document.createElement('div');
tempDiv.innerHTML = '<p>Paste One</p><p>Paste Two</p>';
const parsedDoc = PMDOMParser.fromSchema(schema).parse(tempDiv);
const slice = new Slice(parsedDoc.content, 0, 0);

let tr = state.tr.replaceSelection(slice);
tr.setMeta('inputType', 'insertFromPaste');
const tracked = trackedTransaction({ tr, state, user });
const finalState = state.apply(tracked);

let insertedText = '';
finalState.doc.descendants((node) => {
if (node.isText && node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) {
insertedText += node.text;
}
});

expect(insertedText).toContain('Paste One');
expect(insertedText).toContain('Paste Two');
});

it('tracks plain-text paste insertions', () => {
const doc = schema.nodes.doc.create(
{},
schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Base')])),
);
let state = createState(doc);

const basePos = findTextPos(state.doc, 'Base');
expect(basePos).toBeTypeOf('number');
const insertPos = basePos + 'Base'.length;
state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, insertPos)));

let tr = state.tr.insertText('Plain Paste', insertPos);
tr.setMeta('inputType', 'insertFromPaste');
const tracked = trackedTransaction({ tr, state, user });
const finalState = state.apply(tracked);

let insertedText = '';
finalState.doc.descendants((node) => {
if (node.isText && node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) {
insertedText += node.text;
}
});

expect(insertedText).toContain('Plain Paste');
});

it('tracks paste replacement over selected existing text', () => {
const doc = schema.nodes.doc.create(
{},
schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello World')])),
);
let state = createState(doc);

const worldPos = findTextPos(state.doc, 'Hello World');
expect(worldPos).toBeTypeOf('number');
const from = worldPos + 'Hello '.length;
const to = from + 'World'.length;
state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, from, to)));

const tempDiv = document.createElement('div');
tempDiv.innerHTML = '<p>Pasted</p>';
const parsedDoc = PMDOMParser.fromSchema(schema).parse(tempDiv);
const slice = new Slice(parsedDoc.content, 0, 0);

let tr = state.tr.replaceSelection(slice);
tr.setMeta('inputType', 'insertFromPaste');
const tracked = trackedTransaction({ tr, state, user });
const meta = tracked.getMeta(TrackChangesBasePluginKey);
const finalState = state.apply(tracked);

let insertedText = '';
let deletedText = '';
finalState.doc.descendants((node) => {
if (!node.isText) return;
if (node.marks.some((mark) => mark.type.name === TrackInsertMarkName)) insertedText += node.text;
if (node.marks.some((mark) => mark.type.name === TrackDeleteMarkName)) deletedText += node.text;
});

expect(insertedText).toContain('Pasted');
expect(deletedText).toContain('World');
expect(meta?.insertedMark).toBeDefined();
expect(meta?.deletionMark).toBeDefined();
expect(meta.insertedMark.attrs.id).toBe(meta.deletionMark.attrs.id);
});

it('prefers original paste slice before maxOpen fallback for collapsed insertions', () => {
const doc = schema.nodes.doc.create(
{},
schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Base')])),
);
let state = createState(doc);

const basePos = findTextPos(state.doc, 'Base');
expect(basePos).toBeTypeOf('number');
const insertPos = basePos + 'Base'.length;
state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, insertPos)));

const originalDiv = document.createElement('div');
originalDiv.innerHTML = '<p>Paste One</p><p>Paste Two</p>';
const originalSlice = new Slice(PMDOMParser.fromSchema(schema).parse(originalDiv).content, 0, 0);

const fallbackDiv = document.createElement('div');
fallbackDiv.innerHTML = '<p>Flattened Fallback</p>';
const fallbackSlice = new Slice(PMDOMParser.fromSchema(schema).parse(fallbackDiv).content, 0, 0);
vi.spyOn(Slice, 'maxOpen').mockReturnValue(fallbackSlice);

let tr = state.tr.replaceSelection(originalSlice);
tr.setMeta('inputType', 'insertFromPaste');
const tracked = trackedTransaction({ tr, state, user });
const finalState = state.apply(tracked);

const text = finalState.doc.textBetween(0, finalState.doc.content.size, '\n');
expect(text).toContain('Paste One');
expect(text).toContain('Paste Two');
expect(text).not.toContain('Flattened Fallback');
});

it('tracks replace even when selection contains existing deletions and links', () => {
const linkMark = schema.marks.link.create({ href: 'https://example.com' });
const existingDeletion = schema.marks[TrackDeleteMarkName].create({
Expand Down
Loading