From d5a9671a07efaa408566ac0fd5816a32e011adcd Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Tue, 14 Apr 2026 11:23:53 +0530 Subject: [PATCH 1/4] feat(layout): enable odd/even header-footer support (w:evenAndOddHeaders) Thread alternateHeaders from document settings through the layout engine so the paginator selects the correct header/footer variant per page. Fixes margin calculation for documents with different odd and even page headers. Also fixes getVariantTypeForPage to use document page number (not section-relative) for even/odd selection, matching the rendering side (headerFooterUtils.ts). Closes #2803 --- .../layout-engine/src/index.test.ts | 133 ++++++++++++++++++ .../layout-engine/layout-engine/src/index.ts | 23 ++- .../presentation-editor/PresentationEditor.ts | 8 +- .../v1/core/presentation-editor/types.ts | 1 + 4 files changed, 155 insertions(+), 10 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index b73c27ee11..eefc50e6c3 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -5587,3 +5587,136 @@ describe('requirePageBoundary edge cases', () => { }); }); }); + +describe('alternateHeaders (odd/even header differentiation)', () => { + // Two tall paragraphs (400px each) that force a 2-page layout. + const tallBlock = (id: string): FlowBlock => ({ + kind: 'paragraph', + id, + runs: [], + }); + const tallMeasure = makeMeasure([400]); + + it('selects even/odd header heights when alternateHeaders is true', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + headerContentHeights: { + odd: 80, // Odd pages: header pushes body start down + even: 40, // Even pages: smaller header + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Page 1 is odd (documentPageNumber=1) → uses 'odd' header height (80px) + // Body should start at max(margin.top, margin.header + headerContentHeight) = max(50, 30+80) = 110 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + expect(p1Fragment).toBeDefined(); + expect(p1Fragment!.y).toBeCloseTo(110, 0); + + // Page 2 is even (documentPageNumber=2) → uses 'even' header height (40px) + // Body should start at max(margin.top, margin.header + headerContentHeight) = max(50, 30+40) = 70 + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p2Fragment).toBeDefined(); + expect(p2Fragment!.y).toBeCloseTo(70, 0); + }); + + it('uses default header height for all pages when alternateHeaders is false', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: false, + headerContentHeights: { + default: 60, + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Both pages use 'default' header height (60px) + // Body start = max(50, 30+60) = 90 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + }); + + it('defaults to false when alternateHeaders is omitted', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + // alternateHeaders not set + headerContentHeights: { + default: 60, + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Both pages should use 'default' (60px), not odd/even + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + }); + + it('first page uses first variant when titlePg is enabled with alternateHeaders', () => { + const sectionBreak: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0, titlePg: true }], + headerContentHeights: { + first: 100, // First page: tallest header + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sectionBreak, tallBlock('p1'), tallBlock('p2'), tallBlock('p3')], + [{ kind: 'sectionBreak' }, tallMeasure, tallMeasure, tallMeasure], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(3); + + // Page 1 (first page of section, titlePg=true) → 'first' variant → 100px + // Body start = max(50, 30+100) = 130 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + expect(p1Fragment).toBeDefined(); + expect(p1Fragment!.y).toBeCloseTo(130, 0); + + // Page 2 (documentPageNumber=2, even) → 'even' variant → 40px + // Body start = max(50, 30+40) = 70 + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p2Fragment).toBeDefined(); + expect(p2Fragment!.y).toBeCloseTo(70, 0); + + // Page 3 (documentPageNumber=3, odd) → 'odd' variant → 80px + // Body start = max(50, 30+80) = 110 + const p3Fragment = layout.pages[2].fragments.find((f) => f.blockId === 'p3'); + expect(p3Fragment).toBeDefined(); + expect(p3Fragment!.y).toBeCloseTo(110, 0); + }); +}); diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 32a2aea6c1..649ea274dd 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -528,6 +528,13 @@ export type LayoutOptions = { * behavior for paragraph-free overlays. */ allowSectionBreakOnlyPageFallback?: boolean; + /** + * Whether the document has odd/even header/footer differentiation enabled. + * Corresponds to the w:evenAndOddHeaders element in OOXML settings.xml. + * When true, odd pages use the 'odd' variant and even pages use the 'even' variant. + * When false or omitted, all pages use the 'default' variant. + */ + alternateHeaders?: boolean; }; export type HeaderFooterConstraints = { @@ -669,13 +676,15 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options /** * Determines the header/footer variant type for a given page based on section settings. * - * @param sectionPageNumber - The page number within the current section (1-indexed) + * @param sectionPageNumber - The page number within the current section (1-indexed), used for titlePg + * @param documentPageNumber - The absolute document page number (1-indexed), used for even/odd * @param titlePgEnabled - Whether the section has "different first page" enabled - * @param alternateHeaders - Whether the section has odd/even differentiation enabled + * @param alternateHeaders - Whether the document has odd/even differentiation enabled * @returns The variant type: 'first', 'even', 'odd', or 'default' */ const getVariantTypeForPage = ( sectionPageNumber: number, + documentPageNumber: number, titlePgEnabled: boolean, alternateHeaders: boolean, ): 'default' | 'first' | 'even' | 'odd' => { @@ -683,9 +692,10 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options if (sectionPageNumber === 1 && titlePgEnabled) { return 'first'; } - // Alternate headers (even/odd differentiation) + // Alternate headers: even/odd based on document page number, matching + // the rendering side (getHeaderFooterTypeForSection in headerFooterUtils.ts) if (alternateHeaders) { - return sectionPageNumber % 2 === 0 ? 'even' : 'odd'; + return documentPageNumber % 2 === 0 ? 'even' : 'odd'; } return 'default'; }; @@ -1295,11 +1305,10 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Get section metadata for titlePg setting const sectionMetadata = sectionMetadataList[activeSectionIndex]; const titlePgEnabled = sectionMetadata?.titlePg ?? false; - // TODO: Support alternateHeaders (odd/even) when needed - const alternateHeaders = false; + const alternateHeaders = options.alternateHeaders ?? false; // Determine which header/footer variant applies to this page - const variantType = getVariantTypeForPage(sectionPageNumber, titlePgEnabled, alternateHeaders); + const variantType = getVariantTypeForPage(sectionPageNumber, newPageNumber, titlePgEnabled, alternateHeaders); // Resolve header/footer refs for margin calculation using OOXML inheritance model. // This must match the rendering logic in PresentationEditor to ensure margins 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 64498efb5d..b55bfa4dfa 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 @@ -4238,10 +4238,12 @@ export class PresentationEditor extends EventEmitter { ? buildSemanticFootnoteBlocks(footnotesLayoutInput, this.#layoutOptions.semanticOptions?.footnotesMode) : []; const blocksForLayout = semanticFootnoteBlocks.length > 0 ? [...blocks, ...semanticFootnoteBlocks] : blocks; - const layoutOptions = - !isSemanticFlow && footnotesLayoutInput + const layoutOptions = { + ...(!isSemanticFlow && footnotesLayoutInput ? { ...baseLayoutOptions, footnotes: footnotesLayoutInput } - : baseLayoutOptions; + : baseLayoutOptions), + alternateHeaders: Boolean((this.#editor as EditorWithConverter).converter?.pageStyles?.alternateHeaders), + }; const previousBlocks = this.#layoutState.blocks; const previousLayout = this.#layoutState.layout; const previousMeasures = this.#layoutState.measures; diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts index 0328fcb44c..e442a6f34e 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/types.ts @@ -122,6 +122,7 @@ export type ResolvedLayoutOptions = margins: ResolvedMarginsBase; columns?: { count: number; gap: number }; sectionMetadata: SectionMetadata[]; + alternateHeaders?: boolean; } | { flowMode: 'semantic'; From 1cbd9bcedab4e28bb6a451f4fbbc942096946549 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Tue, 21 Apr 2026 11:10:06 +0530 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20?= =?UTF-8?q?type=20guard=20+=20multi-section/footer/fallback=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Guard alternateHeaders behind isSemanticFlow check to match ResolvedLayoutOptions type (paginated-only field) - Add multi-section test: section 2 starting on page 4 (even by document number, verifies the documentPageNumber fix) - Add footer test: even/odd footer heights with alternateHeaders - Add default-only fallback test: only default header defined --- .../layout-engine/src/index.test.ts | 100 ++++++++++++++++++ .../presentation-editor/PresentationEditor.ts | 6 +- 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index eefc50e6c3..946b00e421 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -5719,4 +5719,104 @@ describe('alternateHeaders (odd/even header differentiation)', () => { expect(p3Fragment).toBeDefined(); expect(p3Fragment!.y).toBeCloseTo(110, 0); }); + + it('multi-section: uses document page number for even/odd, not section-relative', () => { + // Section 1 has 3 pages (pages 1-3), section 2 starts on page 4. + // Page 4 is even by document number, but sectionPageNumber=1 (odd). + // The fix ensures document page number is used for even/odd. + const sb1: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb1', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + const sb2: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb2', + type: 'nextPage', + attrs: { source: 'sectPr', sectionIndex: 1 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0 }, { sectionIndex: 1 }], + headerContentHeights: { + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sb1, tallBlock('p1'), tallBlock('p2'), tallBlock('p3'), sb2, tallBlock('p4')], + [{ kind: 'sectionBreak' }, tallMeasure, tallMeasure, tallMeasure, { kind: 'sectionBreak' }, tallMeasure], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(4); + + // Page 4 (documentPageNumber=4, even) → should use 'even' header (40px) + // NOT 'odd' which would happen if sectionPageNumber (1) were used + // Body start = max(50, 30+40) = 70 + const p4Fragment = layout.pages[3]?.fragments.find((f) => f.blockId === 'p4'); + expect(p4Fragment).toBeDefined(); + expect(p4Fragment!.y).toBeCloseTo(70, 0); + }); + + it('selects even/odd footer heights when alternateHeaders is true', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, footer: 30 }, + alternateHeaders: true, + footerContentHeights: { + odd: 80, // Odd pages: larger footer + even: 40, // Even pages: smaller footer + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Page 1 (odd): effective bottom margin = max(50, 30+80) = 110 + // Content area = 800 - 50 - 110 = 640px + // Page 2 (even): effective bottom margin = max(50, 30+40) = 70 + // Content area = 800 - 50 - 70 = 680px + // Both pages fit a 400px block, but body start Y differs by footer impact + // Verify pages have different available content areas by checking fragment positions + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment).toBeDefined(); + expect(p2Fragment).toBeDefined(); + // Top margin is the same (50) since no header heights + expect(p1Fragment!.y).toBeCloseTo(50, 0); + expect(p2Fragment!.y).toBeCloseTo(50, 0); + }); + + it('falls back to default header when only default is defined with alternateHeaders', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + headerContentHeights: { + default: 60, // Only default defined — no even/odd specific heights + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // With alternateHeaders=true, page 1 requests 'odd' variant, page 2 requests 'even'. + // headerContentHeights has no 'odd' or 'even' → getHeaderHeightForPage returns 0. + // Body start = max(50, 30+0) = 50 (base margin wins) + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(50, 0); + expect(p2Fragment!.y).toBeCloseTo(50, 0); + }); }); 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 b55bfa4dfa..d78de7407a 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 @@ -4242,7 +4242,11 @@ export class PresentationEditor extends EventEmitter { ...(!isSemanticFlow && footnotesLayoutInput ? { ...baseLayoutOptions, footnotes: footnotesLayoutInput } : baseLayoutOptions), - alternateHeaders: Boolean((this.#editor as EditorWithConverter).converter?.pageStyles?.alternateHeaders), + ...(isSemanticFlow + ? {} + : { + alternateHeaders: Boolean((this.#editor as EditorWithConverter).converter?.pageStyles?.alternateHeaders), + }), }; const previousBlocks = this.#layoutState.blocks; const previousLayout = this.#layoutState.layout; From 6e8898dbcf413e728905483d7915440e951c625f Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 21 Apr 2026 07:50:03 -0300 Subject: [PATCH 3/4] test(layout): strengthen alternateHeaders tests and thread flag via resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 follow-ups on PR #2804. Tests - Footer test now goes through sectionMetadata.footerRefs + footerContentHeightsByRId (the real path) and asserts page.margins.bottom. The old test only checked body-top y, which is footer-independent — stubbing getFooterHeightForPage to always return 0 left that assertion passing. Mutation-tested: forcing getVariantTypeForPage to always return 'default' now breaks it. - Default-only fallback test now exercises the production path (headerRefs.default + per-rId heights) and asserts the correct outcome (y ~= 90 via the step-3 default fallback). Old assertion of y ~= 50 codified a code path that never runs in production because document imports always supply section refs. Mutation-tested: removing the step-3 fallback breaks this test. - New combined test: multi-section + titlePg + alternateHeaders, where section 2 has titlePg=true and starts on an even document page. Guards both the titlePg interaction across a section boundary AND the documentPageNumber (not sectionPageNumber) rule on pages 5 and 6. Mutation-tested: reverting to sectionPageNumber breaks this test alongside the original multi-section case. Layout engine - getVariantTypeForPage now takes a named-params object. Two adjacent `number` params (sectionPageNumber, documentPageNumber) are swap-vulnerable. - JSDoc on LayoutOptions.alternateHeaders cross-references getHeaderFooterTypeForSection in layout-bridge — the two sides must agree on variant selection and the pointer helps future maintainers keep them in sync. PresentationEditor - alternateHeaders is now populated inside #resolveLayoutOptions, alongside the other paginated-only fields (columns, sectionMetadata). The render-site spread collapses back to the single ternary it was before, and the `as EditorWithConverter` cast there disappears. types.ts didn't need changes — the field was already declared on the paginated variant of ResolvedLayoutOptions but unpopulated; it's now legitimately set by the resolver. --- .../layout-engine/src/index.test.ts | 126 ++++++++++++++---- .../layout-engine/layout-engine/src/index.ts | 32 +++-- .../presentation-editor/PresentationEditor.ts | 17 ++- 3 files changed, 131 insertions(+), 44 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index 946b00e421..6e4d3204ef 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -5768,55 +5768,131 @@ describe('alternateHeaders (odd/even header differentiation)', () => { }); it('selects even/odd footer heights when alternateHeaders is true', () => { + // The footer-height path uses the per-rId map + sectionMetadata.footerRefs. + // Exposing the variant selection through `footerContentHeights` alone is not + // sufficient — without refs, the code falls back to 'default' for the footer + // variant regardless. We need the ref map to observe variant switching on + // `page.margins.bottom`. const options: LayoutOptions = { pageSize: { w: 600, h: 800 }, margins: { top: 50, right: 50, bottom: 50, left: 50, footer: 30 }, alternateHeaders: true, - footerContentHeights: { - odd: 80, // Odd pages: larger footer - even: 40, // Even pages: smaller footer - }, + sectionMetadata: [{ sectionIndex: 0, footerRefs: { odd: 'rIdFooterOdd', even: 'rIdFooterEven' } }], + footerContentHeightsByRId: new Map([ + ['rIdFooterOdd', 80], // Odd pages: larger footer + ['rIdFooterEven', 40], // Even pages: smaller footer + ]), }; const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); expect(layout.pages).toHaveLength(2); - // Page 1 (odd): effective bottom margin = max(50, 30+80) = 110 - // Content area = 800 - 50 - 110 = 640px - // Page 2 (even): effective bottom margin = max(50, 30+40) = 70 - // Content area = 800 - 50 - 70 = 680px - // Both pages fit a 400px block, but body start Y differs by footer impact - // Verify pages have different available content areas by checking fragment positions - const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); - const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); - expect(p1Fragment).toBeDefined(); - expect(p2Fragment).toBeDefined(); - // Top margin is the same (50) since no header heights - expect(p1Fragment!.y).toBeCloseTo(50, 0); - expect(p2Fragment!.y).toBeCloseTo(50, 0); + // Page 1 is odd → 'odd' footer (80px) → bottom = max(50, 30+80) = 110 + // Page 2 is even → 'even' footer (40px) → bottom = max(50, 30+40) = 70 + // Body-top Y is footer-independent, so assert on the effective bottom margin + // the paginator stamped on each page. + expect(layout.pages[0].margins?.bottom).toBeCloseTo(110, 0); + expect(layout.pages[1].margins?.bottom).toBeCloseTo(70, 0); }); it('falls back to default header when only default is defined with alternateHeaders', () => { + // Production path: a document with `w:evenAndOddHeaders` on but only a + // `default` header authored. sectionMetadata supplies the `default` ref and + // the per-rId height map supplies its measurement. Step-3 fallback at + // index.ts:1345-1349 kicks in and `effectiveVariantType` drops to 'default'. const options: LayoutOptions = { pageSize: { w: 600, h: 800 }, margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, alternateHeaders: true, - headerContentHeights: { - default: 60, // Only default defined — no even/odd specific heights - }, + sectionMetadata: [{ sectionIndex: 0, headerRefs: { default: 'rIdHeaderDefault' } }], + headerContentHeightsByRId: new Map([['rIdHeaderDefault', 60]]), }; const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); expect(layout.pages).toHaveLength(2); - // With alternateHeaders=true, page 1 requests 'odd' variant, page 2 requests 'even'. - // headerContentHeights has no 'odd' or 'even' → getHeaderHeightForPage returns 0. - // Body start = max(50, 30+0) = 50 (base margin wins) + // Both pages fall back to the default header (60px), so body start is the + // same on odd and even: max(50, 30+60) = 90. const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); - expect(p1Fragment!.y).toBeCloseTo(50, 0); - expect(p2Fragment!.y).toBeCloseTo(50, 0); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + // Effective top margin is also 90 on both pages. + expect(layout.pages[0].margins?.top).toBeCloseTo(90, 0); + expect(layout.pages[1].margins?.top).toBeCloseTo(90, 0); + }); + + it('multi-section + titlePg + alternateHeaders: first page of section 2 lands on an even doc-page', () => { + // Most realistic mixed case. Section 1 has 3 pages (docPN 1-3). Section 2 + // has titlePg=true and starts on docPN=4. + // - Page 4 is sectionPageNumber=1 for section 2 + titlePg=true → 'first' + // - Page 5 is docPN=5 (odd) → 'odd' (regardless of section-relative number) + // - Page 6 is docPN=6 (even) → 'even' + // If the code used sectionPageNumber for even/odd, pages 5 and 6 would be + // swapped (section-relative 2 and 3 respectively). This guards both titlePg + // and the docPN rule across a section boundary. + const sb1: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb1', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + const sb2: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb2', + type: 'nextPage', + attrs: { source: 'sectPr', sectionIndex: 1 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0 }, { sectionIndex: 1, titlePg: true }], + headerContentHeights: { + first: 100, // section 2 title-page header + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sb1, tallBlock('p1'), tallBlock('p2'), tallBlock('p3'), sb2, tallBlock('p4'), tallBlock('p5'), tallBlock('p6')], + [ + { kind: 'sectionBreak' }, + tallMeasure, + tallMeasure, + tallMeasure, + { kind: 'sectionBreak' }, + tallMeasure, + tallMeasure, + tallMeasure, + ], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(6); + + // Page 4: section 2 first page + titlePg → 'first' (100px) → y = max(50, 30+100) = 130 + const p4Fragment = layout.pages[3]?.fragments.find((f) => f.blockId === 'p4'); + expect(p4Fragment).toBeDefined(); + expect(p4Fragment!.y).toBeCloseTo(130, 0); + + // Page 5: docPN=5, odd → 'odd' (80px) → y = max(50, 30+80) = 110 + // If sectionPageNumber were used: sectionPN=2 → 'even' (40) → y = 70 (wrong) + const p5Fragment = layout.pages[4]?.fragments.find((f) => f.blockId === 'p5'); + expect(p5Fragment).toBeDefined(); + expect(p5Fragment!.y).toBeCloseTo(110, 0); + + // Page 6: docPN=6, even → 'even' (40px) → y = max(50, 30+40) = 70 + // If sectionPageNumber were used: sectionPN=3 → 'odd' (80) → y = 110 (wrong) + const p6Fragment = layout.pages[5]?.fragments.find((f) => f.blockId === 'p6'); + expect(p6Fragment).toBeDefined(); + expect(p6Fragment!.y).toBeCloseTo(70, 0); }); }); diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 649ea274dd..413ce32247 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -533,6 +533,10 @@ export type LayoutOptions = { * Corresponds to the w:evenAndOddHeaders element in OOXML settings.xml. * When true, odd pages use the 'odd' variant and even pages use the 'even' variant. * When false or omitted, all pages use the 'default' variant. + * + * Must stay in sync with `getHeaderFooterTypeForSection` in + * `layout-bridge/src/headerFooterUtils.ts` — both sides read this value + * and must agree on variant selection. */ alternateHeaders?: boolean; }; @@ -676,26 +680,29 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options /** * Determines the header/footer variant type for a given page based on section settings. * + * Takes a params object because the two page-number fields have very similar + * names and types — a positional call site is easy to get wrong. + * * @param sectionPageNumber - The page number within the current section (1-indexed), used for titlePg * @param documentPageNumber - The absolute document page number (1-indexed), used for even/odd * @param titlePgEnabled - Whether the section has "different first page" enabled * @param alternateHeaders - Whether the document has odd/even differentiation enabled * @returns The variant type: 'first', 'even', 'odd', or 'default' */ - const getVariantTypeForPage = ( - sectionPageNumber: number, - documentPageNumber: number, - titlePgEnabled: boolean, - alternateHeaders: boolean, - ): 'default' | 'first' | 'even' | 'odd' => { + const getVariantTypeForPage = (args: { + sectionPageNumber: number; + documentPageNumber: number; + titlePgEnabled: boolean; + alternateHeaders: boolean; + }): 'default' | 'first' | 'even' | 'odd' => { // First page of section with titlePg enabled uses 'first' variant - if (sectionPageNumber === 1 && titlePgEnabled) { + if (args.sectionPageNumber === 1 && args.titlePgEnabled) { return 'first'; } // Alternate headers: even/odd based on document page number, matching // the rendering side (getHeaderFooterTypeForSection in headerFooterUtils.ts) - if (alternateHeaders) { - return documentPageNumber % 2 === 0 ? 'even' : 'odd'; + if (args.alternateHeaders) { + return args.documentPageNumber % 2 === 0 ? 'even' : 'odd'; } return 'default'; }; @@ -1308,7 +1315,12 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options const alternateHeaders = options.alternateHeaders ?? false; // Determine which header/footer variant applies to this page - const variantType = getVariantTypeForPage(sectionPageNumber, newPageNumber, titlePgEnabled, alternateHeaders); + const variantType = getVariantTypeForPage({ + sectionPageNumber, + documentPageNumber: newPageNumber, + titlePgEnabled, + alternateHeaders, + }); // Resolve header/footer refs for margin calculation using OOXML inheritance model. // This must match the rendering logic in PresentationEditor to ensure margins 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 d78de7407a..2c9d1dab9d 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 @@ -4238,16 +4238,10 @@ export class PresentationEditor extends EventEmitter { ? buildSemanticFootnoteBlocks(footnotesLayoutInput, this.#layoutOptions.semanticOptions?.footnotesMode) : []; const blocksForLayout = semanticFootnoteBlocks.length > 0 ? [...blocks, ...semanticFootnoteBlocks] : blocks; - const layoutOptions = { - ...(!isSemanticFlow && footnotesLayoutInput + const layoutOptions = + !isSemanticFlow && footnotesLayoutInput ? { ...baseLayoutOptions, footnotes: footnotesLayoutInput } - : baseLayoutOptions), - ...(isSemanticFlow - ? {} - : { - alternateHeaders: Boolean((this.#editor as EditorWithConverter).converter?.pageStyles?.alternateHeaders), - }), - }; + : baseLayoutOptions; const previousBlocks = this.#layoutState.blocks; const previousLayout = this.#layoutState.layout; const previousMeasures = this.#layoutState.measures; @@ -5416,12 +5410,17 @@ export class PresentationEditor extends EventEmitter { this.#hiddenHost.style.width = `${pageSize.w}px`; + const alternateHeaders = Boolean( + (this.#editor as EditorWithConverter | undefined)?.converter?.pageStyles?.alternateHeaders, + ); + return { flowMode: 'paginated', pageSize, margins: resolvedMargins, ...(columns ? { columns } : {}), sectionMetadata, + alternateHeaders, }; } From 7dbf4f6ac27eec6a78fdaac90fdc8444faff5390 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 21 Apr 2026 09:19:28 -0300 Subject: [PATCH 4/4] test(alternate-headers): add unit + integration + behavior coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to round 2 review. Closes the three test gaps flagged in the gap analysis: Unit (PresentationEditor threading) - 3 tests in PresentationEditor.test.ts assert that `converter.pageStyles.alternateHeaders` is forwarded to the layout options that reach `incrementalLayout`. Covers true, unset, and falsy-non-boolean cases. A refactor that drops the threading no longer passes silently. Unit (docxImporter) - Export `isAlternatingHeadersOddEven` and add 4 tests covering the import-side signal: `` present, absent, missing settings.xml, empty settings. Pins the contract between OOXML settings and `pageStyles.alternateHeaders`. Behavior (Playwright) - `tests/headers/odd-even-header-variants.spec.ts` loads `h_f-normal-odd-even.docx` (already in the corpus) and asserts: - pages 1/3 render the default/odd header text, pages 2/4 render the even header text - page 1 and page 3 use the same `data-block-id` (same rId) but differ from page 2 — catches regressions that produce the right text from the wrong rId - footers follow the same alternation The existing layout/visual corpus already includes `h_f-normal-odd-even*.docx` and `sd-2234-even-odd-headers.docx`, so rendering regressions show up in `pnpm test:layout` and `pnpm test:visual` without any additional wiring. --- .../tests/PresentationEditor.test.ts | 58 ++++++++++++ .../v2/importer/docxImporter.js | 2 +- .../v2/importer/docxImporter.test.js | 38 ++++++++ .../headers/odd-even-header-variants.spec.ts | 88 +++++++++++++++++++ 4 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tests/behavior/tests/headers/odd-even-header-variants.spec.ts 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 e7f3e45202..40f9fd8b4e 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 @@ -772,6 +772,64 @@ describe('PresentationEditor', () => { }); }); + describe('alternateHeaders threading (paginated flow)', () => { + it('forwards converter.pageStyles.alternateHeaders=true to incrementalLayout', async () => { + mockEditorConverterStore.current.pageStyles = { alternateHeaders: true }; + + editor = new PresentationEditor({ + element: container, + documentId: 'alt-headers-true-doc', + mode: 'docx', + }); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(mockIncrementalLayout).toHaveBeenCalled(); + const layoutOptions = mockIncrementalLayout.mock.calls[mockIncrementalLayout.mock.calls.length - 1]?.[3] as { + flowMode?: string; + alternateHeaders?: boolean; + }; + expect(layoutOptions.flowMode).toBe('paginated'); + expect(layoutOptions.alternateHeaders).toBe(true); + }); + + it('forwards alternateHeaders=false when converter.pageStyles.alternateHeaders is unset', async () => { + // converter has no pageStyles.alternateHeaders by default + editor = new PresentationEditor({ + element: container, + documentId: 'alt-headers-default-doc', + mode: 'docx', + }); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + const layoutOptions = mockIncrementalLayout.mock.calls[mockIncrementalLayout.mock.calls.length - 1]?.[3] as { + flowMode?: string; + alternateHeaders?: boolean; + }; + expect(layoutOptions.flowMode).toBe('paginated'); + expect(layoutOptions.alternateHeaders).toBe(false); + }); + + 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 }; + + editor = new PresentationEditor({ + element: container, + documentId: 'alt-headers-coerce-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(false); + }); + }); + describe('scrollToPage', () => { const buildMixedPageLayout = () => ({ layout: { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js index f4a2706bf7..f6cb80864b 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js @@ -1241,7 +1241,7 @@ function getNumberingDefinitions(docx, converter) { * @param {Object} docx The parsed docx object * @returns {Boolean} True if the document has alternating headers and footers, false otherwise */ -const isAlternatingHeadersOddEven = (docx) => { +export const isAlternatingHeadersOddEven = (docx) => { const settings = docx['word/settings.xml']; if (!settings || !settings.elements?.length) return false; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.test.js index 889ebfadc9..b64c9c7997 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.test.js @@ -3,6 +3,7 @@ import { collapseWhitespaceNextToInlinePassthrough, defaultNodeListHandler, filterOutRootInlineNodes, + isAlternatingHeadersOddEven, normalizeTableBookmarksInContent, } from './docxImporter.js'; @@ -484,3 +485,40 @@ describe('docPartObj paragraph import regression', () => { expect(result[0].content?.[0]?.content?.[0]).toMatchObject({ type: 'text', text: 'Header text' }); }); }); + +describe('isAlternatingHeadersOddEven', () => { + // Helper: build the same docx-object shape the XML parser produces + const makeSettingsDocx = (settingsChildren) => ({ + 'word/settings.xml': { + elements: [ + { + type: 'element', + name: 'w:settings', + elements: settingsChildren, + }, + ], + }, + }); + + it('returns true when word/settings.xml contains ', () => { + const docx = makeSettingsDocx([{ type: 'element', name: 'w:evenAndOddHeaders' }]); + expect(isAlternatingHeadersOddEven(docx)).toBe(true); + }); + + it('returns false when is absent', () => { + const docx = makeSettingsDocx([ + { type: 'element', name: 'w:zoom', attributes: { 'w:percent': '100' } }, + { type: 'element', name: 'w:defaultTabStop', attributes: { 'w:val': '720' } }, + ]); + expect(isAlternatingHeadersOddEven(docx)).toBe(false); + }); + + it('returns false when word/settings.xml is missing', () => { + expect(isAlternatingHeadersOddEven({})).toBe(false); + }); + + it('returns false when settings has no elements', () => { + expect(isAlternatingHeadersOddEven({ 'word/settings.xml': { elements: [] } })).toBe(false); + expect(isAlternatingHeadersOddEven({ 'word/settings.xml': {} })).toBe(false); + }); +}); diff --git a/tests/behavior/tests/headers/odd-even-header-variants.spec.ts b/tests/behavior/tests/headers/odd-even-header-variants.spec.ts new file mode 100644 index 0000000000..c71eff1656 --- /dev/null +++ b/tests/behavior/tests/headers/odd-even-header-variants.spec.ts @@ -0,0 +1,88 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, test } from '../../fixtures/superdoc.js'; + +// This test verifies the per-page header-variant selection behavior for documents +// with `w:evenAndOddHeaders` enabled. The layout engine picks the correct header +// rId per document page (odd pages use the default/odd header, even pages use the +// even header), and the DomPainter renders the correct header text per page. + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +// `h_f-normal-odd-even.docx` (test-corpus/pagination/) has: +// - settings.xml: +// - header default (rId8) text: "1 | Odd page | header, page numbers at the right" +// - header even (rId7) text: "2 | Even page header, page numbers at the right" +// - 110 paragraphs → many rendered pages +// - No titlePg, so page 1 uses the default (odd) header. +const DOC_PATH = path.resolve(__dirname, '../../test-data/pagination/h_f-normal-odd-even.docx'); + +test.skip(!fs.existsSync(DOC_PATH), 'Test document not available — run pnpm corpus:pull'); + +test('odd/even pages render the correct header variant per document page number', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + + const pages = superdoc.page.locator('.superdoc-page[data-page-number]'); + await expect(pages.first()).toBeVisible({ timeout: 15_000 }); + + // Need at least three pages to observe the odd → even → odd alternation. + await expect.poll(async () => await pages.count(), { timeout: 15_000 }).toBeGreaterThanOrEqual(3); + + const pageCount = await pages.count(); + + // Page 1 — docPN=1, odd, no titlePg → default variant (rId8) → "Odd page" text. + const page1Header = pages.nth(0).locator('.superdoc-page-header'); + await expect(page1Header).toContainText('Odd page'); + await expect(page1Header).not.toContainText('Even page'); + + // Page 2 — docPN=2, even → even variant (rId7) → "Even page" text. + const page2Header = pages.nth(1).locator('.superdoc-page-header'); + await expect(page2Header).toContainText('Even page'); + await expect(page2Header).not.toContainText('Odd page'); + + // Page 3 — docPN=3, odd → default variant again → "Odd page" text. + // This is what confirms the alternation is driven by documentPageNumber, not + // a one-off flag. + const page3Header = pages.nth(2).locator('.superdoc-page-header'); + await expect(page3Header).toContainText('Odd page'); + await expect(page3Header).not.toContainText('Even page'); + + // Spot-check the data-block-id on each header so the test also catches + // regressions that produce the right TEXT but from the wrong rId (e.g., if + // every page linked to the same header fragment). + const page1BlockId = await page1Header.locator('[data-block-id]').first().getAttribute('data-block-id'); + const page2BlockId = await page2Header.locator('[data-block-id]').first().getAttribute('data-block-id'); + const page3BlockId = await page3Header.locator('[data-block-id]').first().getAttribute('data-block-id'); + + expect(page1BlockId, 'page 1 should use a different header fragment than page 2').not.toBe(page2BlockId); + expect(page1BlockId, 'page 1 and page 3 should use the same (default/odd) header fragment').toBe(page3BlockId); + + // Bonus: if the doc produced enough pages, page 4 should alternate back to even. + if (pageCount >= 4) { + const page4Header = pages.nth(3).locator('.superdoc-page-header'); + await expect(page4Header).toContainText('Even page'); + } +}); + +test('footers follow the same odd/even alternation as headers', async ({ superdoc }) => { + await superdoc.loadDocument(DOC_PATH); + await superdoc.waitForStable(); + + const pages = superdoc.page.locator('.superdoc-page[data-page-number]'); + await expect.poll(async () => await pages.count(), { timeout: 15_000 }).toBeGreaterThanOrEqual(3); + + // Footers aren't always in the initial viewport; scroll to each page to make + // sure the footer is mounted before asserting. + for (const [idx, expected] of [ + [0, 'Odd page'], + [1, 'Even'], + [2, 'Odd page'], + ] as const) { + const pageEl = pages.nth(idx); + await pageEl.scrollIntoViewIfNeeded(); + const footer = pageEl.locator('.superdoc-page-footer'); + await expect(footer).toContainText(expected); + } +});