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 @@ -51,6 +51,24 @@ export const calculateInlineRunPropertiesPlugin = (editor) =>
const runType = newState.schema.nodes.run;
if (!runType) return null;

// Track-change accept/reject rewrites marks to a canonical state. Inline-key metadata
// and run.runProperties from the prior (suggested) state are stale and must not be
// re-applied by the SD-2517 lost-keys preservation below.
const isAcceptReject = transactions.some((t) => t.getMeta('inputType') === 'acceptReject');

// Collect mark types the user explicitly removed in this batch — for these, the
// lost-keys preservation must NOT re-apply values from stale run.runProperties.
// Map textStyle attr-keys to 'textStyle' for comparison.
const removedMarkTypes = new Set();
transactions.forEach((t) => {
t.steps.forEach((step) => {
const jsonStep = step.toJSON?.();
if (jsonStep?.stepType === 'removeMark' && jsonStep.mark?.type) {
removedMarkTypes.add(jsonStep.mark.type);
}
});
});

const preservedDerivedKeys = new Set();
const preferExistingKeys = new Set();
transactions.forEach((transaction) => {
Expand Down Expand Up @@ -154,16 +172,25 @@ export const calculateInlineRunPropertiesPlugin = (editor) =>
// dropped some of those keys (e.g. fontFamily "matches" the style due to
// mark round-trip comparison), preserve the original keys. The importer saw
// explicit w:rPr in the XML and that decision is authoritative. (SD-2517)
if (hadInlineKeys) {
//
// Skip entirely during accept/reject: the restored marks are canonical, and
// existing run.runProperties still reflect the pre-resolution (suggested) state.
if (hadInlineKeys && !isAcceptReject) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve unaffected inline keys during accept/reject

The new !isAcceptReject guard disables lost-key preservation for all inline keys whenever any accept/reject transaction runs, not just for keys that are actually stale from suggestion marks. In imported docs where explicit w:rPr keys can be dropped by style-equivalence comparison, accepting/rejecting a change now strips those keys from runProperties; exporter logic only writes keys that still exist in runProperties, so the original explicit formatting can be lost on round-trip export.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — narrowing the check so it only skips fields that were actually part of the removed marks, not all of them. Follow-up PR incoming.

const computedKeys = new Set(runProperties ? Object.keys(runProperties) : []);
const lostKeys = existingInlineKeys.filter((k) => !computedKeys.has(k));
if (lostKeys.length > 0) {
if (!runProperties) runProperties = {};
lostKeys.forEach((k) => {
// If the user just removed the standalone mark for this key, don't
// preserve the stale value from the run node's runProperties.
// (For textStyle-derived keys, preserve — the import w:rPr is authoritative
// and in-batch textStyle rewrites replace the mark rather than strip a single attr.)
if (removedMarkTypes.has(k)) return;
if (runNode.attrs?.runProperties?.[k] !== undefined) {
runProperties[k] = runNode.attrs.runProperties[k];
}
});
if (runProperties && Object.keys(runProperties).length === 0) runProperties = null;
}
}
const { inlineKeys: newInlineKeys, overrideKeys: newOverrideKeys } = computeSegmentKeys(
Expand Down
5 changes: 2 additions & 3 deletions tests/behavior/tests/navigation/extract.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { test, expect } from '../../fixtures/superdoc.js';
import { addCommentByText, replaceText, findFirstTextRange } from '../../helpers/document-api.js';
import { addCommentByText, replaceText, findFirstSelectionTarget } from '../../helpers/document-api.js';

test('@behavior SD-2525: doc.extract returns blocks with nodeIds and full text', async ({ superdoc }) => {
await superdoc.type('Hello world');
Expand Down Expand Up @@ -37,7 +37,6 @@ test('@behavior SD-2525: doc.extract returns empty arrays when no comments or tr
});

test('@behavior SD-2525: doc.extract returns full text not truncated', async ({ superdoc }) => {
await superdoc.click();
const longText =
'This is a long paragraph that exceeds eighty characters to verify text is not truncated like textPreview is.';
await superdoc.type(longText);
Expand Down Expand Up @@ -93,7 +92,7 @@ test('@behavior SD-2525: doc.extract returns comments with entityId and blockId'
test('@behavior SD-2525: doc.extract returns tracked changes', async ({ superdoc }) => {
await superdoc.type('Original text here');

const target = await findFirstTextRange(superdoc.page, 'Original');
const target = await findFirstSelectionTarget(superdoc.page, 'Original');
if (!target) throw new Error('Could not find text range');
await replaceText(superdoc.page, { target, text: 'Modified' }, { changeMode: 'tracked' });

Expand Down
Loading