From c3b1a36812312a060fa7a85aae4dada924fa62d1 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 7 May 2026 17:49:39 +0300 Subject: [PATCH] fix: missing headers in collab mode; wrong odd/even headers --- .../src/editors/v1/core/Editor.ts | 7 + .../presentation-editor/PresentationEditor.ts | 86 ++++++++-- .../PresentationEditor.collaboration.test.ts | 149 +++++++++++++----- .../tests/PresentationEditor.test.ts | 36 +++++ .../src/editors/v1/core/types/EditorEvents.ts | 12 ++ 5 files changed, 240 insertions(+), 50 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index 60777862ae..df697952dc 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -3892,6 +3892,13 @@ export class Editor extends EventEmitter { if (!this.options.ydoc) { this.#initComments(); } + + // AIDEV-NOTE: In collaboration mode, parts are seeded into Y.Doc directly + // (not through `mutateParts`), so no `partChanged` fires on this client. + // Remote tabs refresh via the consumer → `mutateParts` → `partChanged` path, + // but the importer relies on this signal to rebuild header/footer state + // bound to the previous document (SD-2643). + this.emit('documentReplaced', { editor: this }); } /** diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index 2a79f4ea31..8f62a40e86 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -4533,6 +4533,14 @@ export class PresentationEditor extends EventEmitter { const handleCollaborationReady = (payload: unknown) => { this.emit('collaborationReady', payload); + // Collaboration bootstrap can hydrate header/footer parts on this client + // without emitting partChanged. Force a header/footer refresh pass so the + // importer tab sees the same headers/footers immediately. + this.#headerFooterSession?.refreshStructure(); + this.#flowBlockCache.setHasExternalChanges(true); + this.#pendingDocChange = true; + this.#selectionSync.onLayoutStart(); + this.#scheduleRerender(); // Setup remote cursor rendering after collaboration is ready // Only setup if presence is enabled in layout options if (this.#options.collaborationProvider?.awareness && this.#layoutOptions.presence?.enabled !== false) { @@ -4544,6 +4552,24 @@ export class PresentationEditor extends EventEmitter { event: 'collaborationReady', handler: handleCollaborationReady as (...args: unknown[]) => void, }); + + // `Editor.replaceFile()` swaps the converter and (in collaboration mode) + // seeds parts straight into the Y.Doc, so no `partChanged` fires on the + // importing client. Treat the signal like a structural rels change: rebuild + // header/footer descriptors against the new converter and rerender so the + // importer tab matches the collaborator tab without waiting for an edit. + const handleDocumentReplaced = () => { + this.#headerFooterSession?.refreshStructure(); + this.#flowBlockCache.setHasExternalChanges(true); + this.#pendingDocChange = true; + this.#selectionSync.onLayoutStart(); + this.#scheduleRerender(); + }; + this.#editor.on('documentReplaced', handleDocumentReplaced); + this.#editorListeners.push({ + event: 'documentReplaced', + handler: handleDocumentReplaced as (...args: unknown[]) => void, + }); // Listen for comment selection changes and re-run the inline style layering // pipeline on the existing DOM. This avoids a full layout → paint cycle // while still restoring bridge-owned inline decoration styles afterward. @@ -6206,13 +6232,19 @@ export class PresentationEditor extends EventEmitter { } this.#sectionMetadata = sectionMetadata; - // Build multi-section identifier from section metadata for section-aware header/footer selection - // Pass converter's headerIds/footerIds as fallbacks for dynamically created headers/footers + // Build multi-section identifier from section metadata for section-aware header/footer selection. + // Derive odd/even mode from current settings.xml-aware resolution (not only converter.pageStyles), + // because collaborator sessions can have stale converter.pageStyles during remote hydration. + // Pass converter's headerIds/footerIds as fallbacks for dynamically created headers/footers. const converter = (this.#editor as EditorWithConverter).converter; - const multiSectionId = buildMultiSectionIdentifier(sectionMetadata, converter?.pageStyles, { - headerIds: converter?.headerIds, - footerIds: converter?.footerIds, - }); + const multiSectionId = buildMultiSectionIdentifier( + sectionMetadata, + { alternateHeaders: this.#resolveAlternateHeadersFlag() }, + { + headerIds: converter?.headerIds, + footerIds: converter?.footerIds, + }, + ); if (this.#headerFooterSession) { this.#headerFooterSession.multiSectionIdentifier = multiSectionId; } @@ -7186,6 +7218,44 @@ export class PresentationEditor extends EventEmitter { overlay.appendChild(fragment); } + #resolveAlternateHeadersFlag(): boolean { + type XmlLikeNode = { + name?: string; + attributes?: Record; + elements?: XmlLikeNode[]; + }; + + const toXmlNode = (value: unknown): XmlLikeNode | null => + value && typeof value === 'object' ? (value as XmlLikeNode) : null; + + const converter = (this.#editor as EditorWithConverter | undefined)?.converter; + if (!converter) { + return false; + } + + const settingsPart = toXmlNode( + (converter as { convertedXml?: Record }).convertedXml?.['word/settings.xml'], + ); + if (!settingsPart) { + return converter.pageStyles?.alternateHeaders === true; + } + + const settingsRoot = + settingsPart?.name === 'w:settings' + ? settingsPart + : settingsPart?.elements?.find((entry) => entry.name === 'w:settings'); + const evenOddNode = settingsRoot?.elements?.find((entry) => entry?.name === 'w:evenAndOddHeaders'); + if (evenOddNode) { + const rawVal = evenOddNode.attributes?.['w:val']; + if (rawVal == null) { + return true; + } + return ['1', 'true', 'on'].includes(String(rawVal).trim().toLowerCase()); + } + + return converter.pageStyles?.alternateHeaders === true; + } + #resolveLayoutOptions(blocks: FlowBlock[] | undefined, sectionMetadata: SectionMetadata[]): ResolvedLayoutOptions { const defaults = this.#computeDefaultLayoutDefaults(); const firstSection = blocks?.find( @@ -7263,9 +7333,7 @@ export class PresentationEditor extends EventEmitter { this.#hiddenHost.style.width = `${pageSize.w}px`; - const alternateHeaders = Boolean( - (this.#editor as EditorWithConverter | undefined)?.converter?.pageStyles?.alternateHeaders, - ); + const alternateHeaders = this.#resolveAlternateHeadersFlag(); return { flowMode: 'paginated', diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.collaboration.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.collaboration.test.ts index c8a7345f99..5b7927f9ff 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.collaboration.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.collaboration.test.ts @@ -1,51 +1,59 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { PresentationEditor } from '../PresentationEditor.js'; +import { Editor } from '../../Editor'; import type { Awareness } from 'y-protocols/awareness'; // Create hoisted mocks -const { mockEditorConverterStore, mockAbsolutePositionToRelativePosition, mockRelativePositionToAbsolutePosition } = - vi.hoisted(() => { - const createDefaultConverter = () => ({ - headers: { - 'rId-header-default': { type: 'doc', content: [{ type: 'paragraph' }] }, - }, - footers: { - 'rId-footer-default': { type: 'doc', content: [{ type: 'paragraph' }] }, - }, - headerIds: { - default: 'rId-header-default', - first: null, - even: null, - odd: null, - ids: ['rId-header-default'], - }, - footerIds: { - default: 'rId-footer-default', - first: null, - even: null, - odd: null, - ids: ['rId-footer-default'], - }, - }); - - const converterStore = { - current: createDefaultConverter() as ReturnType & Record, - mediaFiles: {} as Record, - }; - - return { - mockEditorConverterStore: converterStore, - mockAbsolutePositionToRelativePosition: vi.fn((pos) => ({ type: 'relative', pos })), - mockRelativePositionToAbsolutePosition: vi.fn((relPos) => { - if (relPos == null) return null; - if (typeof relPos === 'object' && 'pos' in relPos) { - return (relPos as { pos: number }).pos; - } - return null; - }), - }; +const { + mockEditorConverterStore, + mockAbsolutePositionToRelativePosition, + mockRelativePositionToAbsolutePosition, + mockHeaderFooterRefresh, + mockHeaderFooterInvalidateAll, +} = vi.hoisted(() => { + const createDefaultConverter = () => ({ + headers: { + 'rId-header-default': { type: 'doc', content: [{ type: 'paragraph' }] }, + }, + footers: { + 'rId-footer-default': { type: 'doc', content: [{ type: 'paragraph' }] }, + }, + headerIds: { + default: 'rId-header-default', + first: null, + even: null, + odd: null, + ids: ['rId-header-default'], + }, + footerIds: { + default: 'rId-footer-default', + first: null, + even: null, + odd: null, + ids: ['rId-footer-default'], + }, }); + const converterStore = { + current: createDefaultConverter() as ReturnType & Record, + mediaFiles: {} as Record, + }; + + return { + mockEditorConverterStore: converterStore, + mockAbsolutePositionToRelativePosition: vi.fn((pos) => ({ type: 'relative', pos })), + mockRelativePositionToAbsolutePosition: vi.fn((relPos) => { + if (relPos == null) return null; + if (typeof relPos === 'object' && 'pos' in relPos) { + return (relPos as { pos: number }).pos; + } + return null; + }), + mockHeaderFooterRefresh: vi.fn(), + mockHeaderFooterInvalidateAll: vi.fn(), + }; +}); + // Mock Editor class vi.mock('../../Editor', () => ({ Editor: vi.fn().mockImplementation(() => ({ @@ -145,6 +153,7 @@ vi.mock('../../header-footer/HeaderFooterRegistry', () => ({ createEditor: vi.fn(), destroyEditor: vi.fn(), getEditor: vi.fn(), + refresh: mockHeaderFooterRefresh, on: vi.fn(), off: vi.fn(), destroy: vi.fn(), @@ -153,6 +162,7 @@ vi.mock('../../header-footer/HeaderFooterRegistry', () => ({ clear: vi.fn(), getBatch: vi.fn(() => []), getBlocksByRId: vi.fn(() => new Map()), + invalidateAll: mockHeaderFooterInvalidateAll, setTrackedChangesRenderConfig: vi.fn(), })), })); @@ -232,6 +242,63 @@ describe('PresentationEditor - Collaboration Cursor Throttle', () => { vi.clearAllMocks(); }); + it('refreshes header/footer caches when collaboration becomes ready', () => { + editor = new PresentationEditor({ + element: container, + collaborationProvider: { + awareness: mockAwareness, + }, + }); + + const editorCtorMock = ( + Editor as unknown as { + mock: { results: Array<{ value: { on: ReturnType } }> }; + } + ).mock; + const mockEditorInstance = editorCtorMock.results[editorCtorMock.results.length - 1]?.value; + const onCalls = mockEditorInstance?.on as ReturnType; + const collabReadyHandler = onCalls?.mock?.calls?.find((call) => call[0] === 'collaborationReady')?.[1]; + + expect(typeof collabReadyHandler).toBe('function'); + collabReadyHandler?.(); + + expect(mockHeaderFooterRefresh).toHaveBeenCalled(); + expect(mockHeaderFooterInvalidateAll).toHaveBeenCalled(); + }); + + // Regression: SD-2643 — when the importer tab calls Editor.replaceFile(), parts + // are seeded straight into the Y.Doc without firing partChanged on the local + // editor. PresentationEditor must rebuild header/footer state on the + // documentReplaced signal so the new headers show without waiting for the + // collaborator to make an edit. + it('refreshes header/footer caches when the document is replaced', () => { + editor = new PresentationEditor({ + element: container, + collaborationProvider: { + awareness: mockAwareness, + }, + }); + + const editorCtorMock = ( + Editor as unknown as { + mock: { results: Array<{ value: { on: ReturnType } }> }; + } + ).mock; + const mockEditorInstance = editorCtorMock.results[editorCtorMock.results.length - 1]?.value; + const onCalls = mockEditorInstance?.on as ReturnType; + const documentReplacedHandler = onCalls?.mock?.calls?.find((call) => call[0] === 'documentReplaced')?.[1]; + + expect(typeof documentReplacedHandler).toBe('function'); + + mockHeaderFooterRefresh.mockClear(); + mockHeaderFooterInvalidateAll.mockClear(); + + documentReplacedHandler?.(); + + expect(mockHeaderFooterRefresh).toHaveBeenCalled(); + expect(mockHeaderFooterInvalidateAll).toHaveBeenCalled(); + }); + describe('Race condition fix tests', () => { it('should defer cursor normalization using queueMicrotask', async () => { const queueMicrotaskSpy = vi.spyOn(global, 'queueMicrotask'); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts index 4e0f32a101..d37adab113 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationEditor.test.ts @@ -4,6 +4,7 @@ import { PresentationEditor } from '../PresentationEditor.js'; import type { Editor as EditorInstance } from '../../Editor.js'; import { Editor } from '../../Editor.js'; import { HeaderFooterEditorManager, HeaderFooterLayoutAdapter } from '../../header-footer/HeaderFooterRegistry.js'; +import { buildMultiSectionIdentifier } from '@superdoc/layout-bridge'; type MockedEditor = Mock<(...args: unknown[]) => EditorInstance> & { mock: { @@ -909,6 +910,41 @@ describe('PresentationEditor', () => { expect(layoutOptions.alternateHeaders).toBe(false); }); + it('derives alternateHeaders from word/settings.xml when pageStyles is stale or missing', async () => { + mockEditorConverterStore.current.pageStyles = { alternateHeaders: false }; + mockEditorConverterStore.current.convertedXml = { + ...(mockEditorConverterStore.current.convertedXml ?? {}), + 'word/settings.xml': { + type: 'element', + name: 'document', + elements: [ + { + type: 'element', + name: 'w:settings', + elements: [{ type: 'element', name: 'w:evenAndOddHeaders' }], + }, + ], + }, + }; + + editor = new PresentationEditor({ + element: container, + documentId: 'alt-headers-settings-doc', + mode: 'docx', + }); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + const layoutOptions = mockIncrementalLayout.mock.calls[mockIncrementalLayout.mock.calls.length - 1]?.[3] as { + alternateHeaders?: boolean; + }; + expect(layoutOptions.alternateHeaders).toBe(true); + const sectionIdCall = (buildMultiSectionIdentifier as unknown as Mock).mock.calls.at(-1) as + | [unknown, { alternateHeaders?: boolean }] + | undefined; + expect(sectionIdCall?.[1]?.alternateHeaders).toBe(true); + }); + it('coerces falsy but non-boolean pageStyles.alternateHeaders values to false', async () => { // Defensive check: if a converter emits undefined/null/0/'', we still want a clean boolean mockEditorConverterStore.current.pageStyles = { alternateHeaders: 0 }; diff --git a/packages/super-editor/src/editors/v1/core/types/EditorEvents.ts b/packages/super-editor/src/editors/v1/core/types/EditorEvents.ts index bc84a8edc6..5f0e383af7 100644 --- a/packages/super-editor/src/editors/v1/core/types/EditorEvents.ts +++ b/packages/super-editor/src/editors/v1/core/types/EditorEvents.ts @@ -205,6 +205,18 @@ export interface EditorEventMap extends DefaultEventMap { /** Called when a document is closed via editor.close() */ documentClose: [{ editor: Editor }]; + /** + * Called when the underlying document file has been replaced via `Editor.replaceFile()`. + * + * In collaboration mode, `replaceFile` writes the new converter snapshot directly + * to the Y.Doc parts map without going through the local `mutateParts` pipeline, + * so no `partChanged` event fires on the importing client. Other clients receive + * the parts via the consumer → `mutateParts` → `partChanged` and refresh + * automatically. The importer relies on this signal to refresh derived state + * (such as the header/footer registry) that was bound to the previous document. + */ + documentReplaced: [{ editor: Editor }]; + /** Called when page styles are updated */ pageStyleUpdate: [{ pageMargins?: Record; pageStyles: Record }];