diff --git a/packages/super-editor/src/extensions/chart/chart-immutability-plugin.js b/packages/super-editor/src/extensions/chart/chart-immutability-plugin.js index 433faf29c7..11a43b2115 100644 --- a/packages/super-editor/src/extensions/chart/chart-immutability-plugin.js +++ b/packages/super-editor/src/extensions/chart/chart-immutability-plugin.js @@ -1,4 +1,5 @@ import { Plugin, PluginKey } from 'prosemirror-state'; +import { ySyncPluginKey } from 'y-prosemirror'; export const CHART_IMMUTABILITY_KEY = new PluginKey('chartImmutability'); @@ -122,15 +123,26 @@ export function createChartImmutabilityPlugin() { init(_, state) { return countChartNodes(state.doc); }, - apply(_tr, oldCount) { - // filterTransaction guarantees no chart mutations passed through, - // so the chart count is always unchanged after init. + apply(tr, oldCount, _oldState, newState) { + // Yjs-origin transactions bypass filterTransaction, so the chart + // count may have changed. Recount to keep the fast-path guard + // (oldCount === 0) accurate after collaborative syncs. + if (tr.docChanged && tr.getMeta?.(ySyncPluginKey)) { + // When the document had no charts, only do a full recount if the + // incoming steps actually contain a chart node. This preserves + // O(step slices) cost for text-only remote edits on chart-free docs. + if (oldCount === 0 && !transactionInsertsChart(tr)) { + return 0; + } + return countChartNodes(newState.doc); + } return oldCount; }, }, filterTransaction(tr, state) { if (!tr.docChanged) return true; + if (tr.getMeta?.(ySyncPluginKey)) return true; const oldCount = CHART_IMMUTABILITY_KEY.getState(state) ?? 0; if (oldCount === 0) { diff --git a/packages/super-editor/src/extensions/chart/chart-immutability-plugin.test.js b/packages/super-editor/src/extensions/chart/chart-immutability-plugin.test.js index b332f4c5ee..2d05d4a3e1 100644 --- a/packages/super-editor/src/extensions/chart/chart-immutability-plugin.test.js +++ b/packages/super-editor/src/extensions/chart/chart-immutability-plugin.test.js @@ -1,7 +1,8 @@ import { describe, it, expect } from 'vitest'; -import { Schema } from 'prosemirror-model'; +import { Schema, Slice } from 'prosemirror-model'; import { EditorState } from 'prosemirror-state'; -import { createChartImmutabilityPlugin } from './chart-immutability-plugin.js'; +import { ySyncPluginKey } from 'y-prosemirror'; +import { createChartImmutabilityPlugin, CHART_IMMUTABILITY_KEY } from './chart-immutability-plugin.js'; const schema = new Schema({ nodes: { @@ -107,4 +108,123 @@ describe('chart immutability plugin', () => { const result = state.applyTransaction(tr); expect(result.state.doc.textContent).toContain('typing'); }); + + it('allows remote collaboration replacements that span chart nodes', () => { + const state = createStateWithChart(); + const replacementDoc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('remote'))]); + const tr = state.tr + .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + + const result = state.applyTransaction(tr); + + expect(result.state.doc.textContent).toContain('remote'); + }); + + it('allows snapshot-exit replacements that span chart nodes (no isChangeOrigin)', () => { + const state = createStateWithChart(); + const replacementDoc = schema.nodes.doc.create(null, [ + schema.nodes.paragraph.create(null, schema.text('snapshot exit')), + ]); + // y-prosemirror's unrenderSnapshot() sets { snapshot: null, prevSnapshot: null } + // with no isChangeOrigin flag. + const tr = state.tr + .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null }); + + const result = state.applyTransaction(tr); + + expect(result.state.doc.textContent).toContain('snapshot exit'); + }); + + it('rejects local chart deletion after Yjs sync inserts a chart', () => { + // Start with no charts + const state = createStateWithoutChart(); + + // Simulate Yjs sync that introduces a chart + const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } }); + const paraWithChart = schema.nodes.paragraph.create(null, [schema.text('synced '), chart]); + const syncDoc = schema.nodes.doc.create(null, [paraWithChart]); + const syncTr = state.tr + .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + const synced = state.applyTransaction(syncTr).state; + + // Local attempt to delete the chart must be rejected + const chartPos = findChartPos(synced); + expect(chartPos).toBeGreaterThan(-1); + const deleteTr = synced.tr.delete(chartPos, chartPos + 1); + const result = synced.applyTransaction(deleteTr); + expect(result.state.doc.toString()).toBe(synced.doc.toString()); + }); + + it('rejects local attr change on chart after Yjs sync inserts it', () => { + const state = createStateWithoutChart(); + + // Yjs sync introduces a chart + const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } }); + const paraWithChart = schema.nodes.paragraph.create(null, [chart]); + const syncDoc = schema.nodes.doc.create(null, [paraWithChart]); + const syncTr = state.tr + .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + const synced = state.applyTransaction(syncTr).state; + + // Local setNodeMarkup must be rejected + const chartPos = findChartPos(synced); + const attrTr = synced.tr.setNodeMarkup(chartPos, undefined, { + chartData: { chartType: 'lineChart', series: [] }, + width: 999, + height: 999, + }); + const result = synced.applyTransaction(attrTr); + expect(result.state.doc.toString()).toBe(synced.doc.toString()); + }); + + it('uses fast path after Yjs sync removes all charts', () => { + const state = createStateWithChart(); + + // Yjs sync replaces doc with chart-free content + const plainDoc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('no charts'))]); + const syncTr = state.tr + .replace(0, state.doc.content.size, new Slice(plainDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + const synced = state.applyTransaction(syncTr).state; + + // Local text edits should still work + const textTr = synced.tr.insertText('hello', 1); + const result = synced.applyTransaction(textTr); + expect(result.state.doc.textContent).toContain('hello'); + }); + + it('keeps chart count at 0 for Yjs text-only edits on chart-free docs (fast path)', () => { + const state = createStateWithoutChart(); + expect(CHART_IMMUTABILITY_KEY.getState(state)).toBe(0); + + // Simulate multiple Yjs text-only remote edits — chart count must stay 0 + // without walking the full document each time. + let current = state; + for (let i = 0; i < 5; i++) { + const syncTr = current.tr.insertText(`k${i}`, 1).setMeta(ySyncPluginKey, { isChangeOrigin: true }); + current = current.applyTransaction(syncTr).state; + expect(CHART_IMMUTABILITY_KEY.getState(current)).toBe(0); + } + }); + + it('recounts charts when Yjs sync introduces a chart into a chart-free doc', () => { + const state = createStateWithoutChart(); + expect(CHART_IMMUTABILITY_KEY.getState(state)).toBe(0); + + // Yjs sync introduces a chart + const chart = schema.nodes.chart.create({ chartData: { chartType: 'barChart', series: [] } }); + const paraWithChart = schema.nodes.paragraph.create(null, [schema.text('text '), chart]); + const syncDoc = schema.nodes.doc.create(null, [paraWithChart]); + const syncTr = state.tr + .replace(0, state.doc.content.size, new Slice(syncDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + const synced = state.applyTransaction(syncTr).state; + + // Chart count must update to 1 + expect(CHART_IMMUTABILITY_KEY.getState(synced)).toBe(1); + }); }); diff --git a/packages/super-editor/src/extensions/permission-ranges/permission-ranges.js b/packages/super-editor/src/extensions/permission-ranges/permission-ranges.js index 6e200497a1..cdbc3feb2d 100644 --- a/packages/super-editor/src/extensions/permission-ranges/permission-ranges.js +++ b/packages/super-editor/src/extensions/permission-ranges/permission-ranges.js @@ -1,5 +1,6 @@ import { Plugin, PluginKey } from 'prosemirror-state'; import { Mapping } from 'prosemirror-transform'; +import { ySyncPluginKey } from 'y-prosemirror'; import { Extension } from '@core/Extension.js'; const PERMISSION_PLUGIN_KEY = new PluginKey('permissionRanges'); @@ -304,6 +305,11 @@ export const PermissionRanges = Extension.create({ // Appends transactions to the document to ensure permission ranges are updated. appendTransaction(transactions, oldState, newState) { if (!transactions.some((tr) => tr.docChanged)) return null; + // Any y-prosemirror transaction (remote sync, snapshot enter/exit) + // carries authoritative state — do not attempt to repair permission + // tags against oldState or we will reinsert stale markers and + // corrupt the shared document. + if (transactions.some((tr) => tr.getMeta?.(ySyncPluginKey))) return null; const permTypes = getPermissionTypeInfo(newState.schema); if (!permTypes.startTypes.length || !permTypes.endTypes.length) return null; @@ -370,6 +376,7 @@ export const PermissionRanges = Extension.create({ // Filters transactions to ensure only allowed edits are applied. filterTransaction(tr, state) { if (!tr.docChanged) return true; + if (tr.getMeta?.(ySyncPluginKey)) return true; if (!editor || editor.options.documentMode !== 'viewing') return true; const pluginState = PERMISSION_PLUGIN_KEY.getState(state); if (!pluginState?.hasAllowedRanges) { diff --git a/packages/super-editor/src/extensions/permission-ranges/permission-ranges.test.js b/packages/super-editor/src/extensions/permission-ranges/permission-ranges.test.js index faa95b584c..3ac5c082a5 100644 --- a/packages/super-editor/src/extensions/permission-ranges/permission-ranges.test.js +++ b/packages/super-editor/src/extensions/permission-ranges/permission-ranges.test.js @@ -1,5 +1,7 @@ import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; import { TextSelection } from 'prosemirror-state'; +import { Slice } from 'prosemirror-model'; +import { ySyncPluginKey } from 'y-prosemirror'; import { Editor } from '@core/index.js'; import { SuperConverter } from '@core/super-converter/SuperConverter.js'; @@ -166,6 +168,91 @@ describe('PermissionRanges extension', () => { expect(instance.state.doc.textBetween(editablePos, editablePos + 2)).toContain('Y'); }); + it('allows remote collaboration replacements that span permission ranges', () => { + const instance = createEditor(docWithPermissionRange); + const replacementDoc = instance.schema.nodes.doc.create(null, [ + instance.schema.nodes.paragraph.create(null, [instance.schema.text('Remote replacement')]), + ]); + const tr = instance.state.tr + .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + + instance.view.dispatch(tr); + + expect(instance.state.doc.textContent).toContain('Remote replacement'); + }); + + it('does not reinsert stale permission tags on snapshot-exit replace (no isChangeOrigin)', () => { + const instance = createEditor(docWithPermissionRange); + + // Verify the original doc has permStart and permEnd + let hasPermStart = false; + let hasPermEnd = false; + instance.state.doc.descendants((node) => { + if (node.type?.name === 'permStart') hasPermStart = true; + if (node.type?.name === 'permEnd') hasPermEnd = true; + }); + expect(hasPermStart).toBe(true); + expect(hasPermEnd).toBe(true); + + // Simulate y-prosemirror's unrenderSnapshot() — full-doc replace with + // { snapshot: null, prevSnapshot: null } and NO isChangeOrigin flag. + const replacementDoc = instance.schema.nodes.doc.create(null, [ + instance.schema.nodes.paragraph.create(null, [instance.schema.text('Snapshot exit content')]), + ]); + const tr = instance.state.tr + .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null }); + + instance.view.dispatch(tr); + + // The resulting doc must NOT contain stale permission markers + let permStartCount = 0; + let permEndCount = 0; + instance.state.doc.descendants((node) => { + if (node.type?.name === 'permStart') permStartCount++; + if (node.type?.name === 'permEnd') permEndCount++; + }); + expect(permStartCount).toBe(0); + expect(permEndCount).toBe(0); + expect(instance.state.doc.textContent).toBe('Snapshot exit content'); + }); + + it('does not reinsert stale permission tags when Yjs replaces content that had them', () => { + const instance = createEditor(docWithPermissionRange); + + // Verify the original doc has permStart and permEnd + let hasPermStart = false; + let hasPermEnd = false; + instance.state.doc.descendants((node) => { + if (node.type?.name === 'permStart') hasPermStart = true; + if (node.type?.name === 'permEnd') hasPermEnd = true; + }); + expect(hasPermStart).toBe(true); + expect(hasPermEnd).toBe(true); + + // Simulate a Yjs sync that replaces the whole doc with content lacking perm tags + const replacementDoc = instance.schema.nodes.doc.create(null, [ + instance.schema.nodes.paragraph.create(null, [instance.schema.text('Clean remote content')]), + ]); + const tr = instance.state.tr + .replace(0, instance.state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + + instance.view.dispatch(tr); + + // The resulting doc must NOT contain stale permission markers + let permStartCount = 0; + let permEndCount = 0; + instance.state.doc.descendants((node) => { + if (node.type?.name === 'permStart') permStartCount++; + if (node.type?.name === 'permEnd') permEndCount++; + }); + expect(permStartCount).toBe(0); + expect(permEndCount).toBe(0); + expect(instance.state.doc.textContent).toBe('Clean remote content'); + }); + it('blocks edits outside the block permission range but allows edits inside it', () => { const instance = createEditor(docWithBlockPermissionRange); const initialJson = instance.state.doc.toJSON(); diff --git a/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js b/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js index 578cd6bc85..9750222b9f 100644 --- a/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js +++ b/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js @@ -1,4 +1,5 @@ import { Plugin, PluginKey } from 'prosemirror-state'; +import { ySyncPluginKey } from 'y-prosemirror'; export const STRUCTURED_CONTENT_LOCK_KEY = new PluginKey('structuredContentLock'); @@ -159,6 +160,13 @@ export function createStructuredContentLockPlugin() { return true; } + // Any y-prosemirror transaction (remote sync, snapshot enter/exit) must + // always be applied locally to keep every client converged, even if the + // incoming step spans locked SDTs. + if (tr.getMeta?.(ySyncPluginKey)) { + return true; + } + const sdtNodes = STRUCTURED_CONTENT_LOCK_KEY.getState(state); if (sdtNodes.length === 0) { return true; diff --git a/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js b/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js index a6752efc33..77e27e5458 100644 --- a/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js +++ b/packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js @@ -1,5 +1,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { EditorState, TextSelection } from 'prosemirror-state'; +import { Slice } from 'prosemirror-model'; +import { ySyncPluginKey } from 'y-prosemirror'; import { initTestEditor } from '@tests/helpers/helpers.js'; /** @@ -376,6 +378,38 @@ describe('StructuredContentLockPlugin', () => { // Assert: should handle gracefully (exact behavior depends on schema) expect(newState).toBeDefined(); }); + + it('allows remote collaboration replacements that span locked SDTs', () => { + const doc = createDocWithSDT('sdtContentLocked', 'structuredContent'); + const state = applyDocToEditor(doc); + const replacementParagraph = schema.nodes.paragraph.create(null, schema.text('Remote hello world')); + const replacementDoc = schema.nodes.doc.create(null, [replacementParagraph]); + + const tr = state.tr + .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { isChangeOrigin: true }); + + const result = state.applyTransaction(tr); + + expect(result.state.doc.textContent).toContain('Remote hello world'); + }); + + it('allows snapshot-exit replacements that span locked SDTs (no isChangeOrigin)', () => { + const doc = createDocWithSDT('sdtContentLocked', 'structuredContent'); + const state = applyDocToEditor(doc); + const replacementParagraph = schema.nodes.paragraph.create(null, schema.text('Snapshot exit')); + const replacementDoc = schema.nodes.doc.create(null, [replacementParagraph]); + + // y-prosemirror's unrenderSnapshot() sets { snapshot: null, prevSnapshot: null } + // with no isChangeOrigin flag. + const tr = state.tr + .replace(0, state.doc.content.size, new Slice(replacementDoc.content, 0, 0)) + .setMeta(ySyncPluginKey, { snapshot: null, prevSnapshot: null }); + + const result = state.applyTransaction(tr); + + expect(result.state.doc.textContent).toContain('Snapshot exit'); + }); }); describe('lock mode attribute validation', () => {