Skip to content

Commit 6e8898d

Browse files
committed
test(layout): strengthen alternateHeaders tests and thread flag via resolver
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.
1 parent 1cbd9bc commit 6e8898d

3 files changed

Lines changed: 131 additions & 44 deletions

File tree

packages/layout-engine/layout-engine/src/index.test.ts

Lines changed: 101 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5768,55 +5768,131 @@ describe('alternateHeaders (odd/even header differentiation)', () => {
57685768
});
57695769

57705770
it('selects even/odd footer heights when alternateHeaders is true', () => {
5771+
// The footer-height path uses the per-rId map + sectionMetadata.footerRefs.
5772+
// Exposing the variant selection through `footerContentHeights` alone is not
5773+
// sufficient — without refs, the code falls back to 'default' for the footer
5774+
// variant regardless. We need the ref map to observe variant switching on
5775+
// `page.margins.bottom`.
57715776
const options: LayoutOptions = {
57725777
pageSize: { w: 600, h: 800 },
57735778
margins: { top: 50, right: 50, bottom: 50, left: 50, footer: 30 },
57745779
alternateHeaders: true,
5775-
footerContentHeights: {
5776-
odd: 80, // Odd pages: larger footer
5777-
even: 40, // Even pages: smaller footer
5778-
},
5780+
sectionMetadata: [{ sectionIndex: 0, footerRefs: { odd: 'rIdFooterOdd', even: 'rIdFooterEven' } }],
5781+
footerContentHeightsByRId: new Map([
5782+
['rIdFooterOdd', 80], // Odd pages: larger footer
5783+
['rIdFooterEven', 40], // Even pages: smaller footer
5784+
]),
57795785
};
57805786

57815787
const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options);
57825788

57835789
expect(layout.pages).toHaveLength(2);
57845790

5785-
// Page 1 (odd): effective bottom margin = max(50, 30+80) = 110
5786-
// Content area = 800 - 50 - 110 = 640px
5787-
// Page 2 (even): effective bottom margin = max(50, 30+40) = 70
5788-
// Content area = 800 - 50 - 70 = 680px
5789-
// Both pages fit a 400px block, but body start Y differs by footer impact
5790-
// Verify pages have different available content areas by checking fragment positions
5791-
const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1');
5792-
const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2');
5793-
expect(p1Fragment).toBeDefined();
5794-
expect(p2Fragment).toBeDefined();
5795-
// Top margin is the same (50) since no header heights
5796-
expect(p1Fragment!.y).toBeCloseTo(50, 0);
5797-
expect(p2Fragment!.y).toBeCloseTo(50, 0);
5791+
// Page 1 is odd → 'odd' footer (80px) → bottom = max(50, 30+80) = 110
5792+
// Page 2 is even → 'even' footer (40px) → bottom = max(50, 30+40) = 70
5793+
// Body-top Y is footer-independent, so assert on the effective bottom margin
5794+
// the paginator stamped on each page.
5795+
expect(layout.pages[0].margins?.bottom).toBeCloseTo(110, 0);
5796+
expect(layout.pages[1].margins?.bottom).toBeCloseTo(70, 0);
57985797
});
57995798

58005799
it('falls back to default header when only default is defined with alternateHeaders', () => {
5800+
// Production path: a document with `w:evenAndOddHeaders` on but only a
5801+
// `default` header authored. sectionMetadata supplies the `default` ref and
5802+
// the per-rId height map supplies its measurement. Step-3 fallback at
5803+
// index.ts:1345-1349 kicks in and `effectiveVariantType` drops to 'default'.
58015804
const options: LayoutOptions = {
58025805
pageSize: { w: 600, h: 800 },
58035806
margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 },
58045807
alternateHeaders: true,
5805-
headerContentHeights: {
5806-
default: 60, // Only default defined — no even/odd specific heights
5807-
},
5808+
sectionMetadata: [{ sectionIndex: 0, headerRefs: { default: 'rIdHeaderDefault' } }],
5809+
headerContentHeightsByRId: new Map([['rIdHeaderDefault', 60]]),
58085810
};
58095811

58105812
const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options);
58115813

58125814
expect(layout.pages).toHaveLength(2);
58135815

5814-
// With alternateHeaders=true, page 1 requests 'odd' variant, page 2 requests 'even'.
5815-
// headerContentHeights has no 'odd' or 'even' → getHeaderHeightForPage returns 0.
5816-
// Body start = max(50, 30+0) = 50 (base margin wins)
5816+
// Both pages fall back to the default header (60px), so body start is the
5817+
// same on odd and even: max(50, 30+60) = 90.
58175818
const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1');
58185819
const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2');
5819-
expect(p1Fragment!.y).toBeCloseTo(50, 0);
5820-
expect(p2Fragment!.y).toBeCloseTo(50, 0);
5820+
expect(p1Fragment!.y).toBeCloseTo(90, 0);
5821+
expect(p2Fragment!.y).toBeCloseTo(90, 0);
5822+
// Effective top margin is also 90 on both pages.
5823+
expect(layout.pages[0].margins?.top).toBeCloseTo(90, 0);
5824+
expect(layout.pages[1].margins?.top).toBeCloseTo(90, 0);
5825+
});
5826+
5827+
it('multi-section + titlePg + alternateHeaders: first page of section 2 lands on an even doc-page', () => {
5828+
// Most realistic mixed case. Section 1 has 3 pages (docPN 1-3). Section 2
5829+
// has titlePg=true and starts on docPN=4.
5830+
// - Page 4 is sectionPageNumber=1 for section 2 + titlePg=true → 'first'
5831+
// - Page 5 is docPN=5 (odd) → 'odd' (regardless of section-relative number)
5832+
// - Page 6 is docPN=6 (even) → 'even'
5833+
// If the code used sectionPageNumber for even/odd, pages 5 and 6 would be
5834+
// swapped (section-relative 2 and 3 respectively). This guards both titlePg
5835+
// and the docPN rule across a section boundary.
5836+
const sb1: SectionBreakBlock = {
5837+
kind: 'sectionBreak',
5838+
id: 'sb1',
5839+
attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 },
5840+
pageSize: { w: 600, h: 800 },
5841+
margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 },
5842+
};
5843+
const sb2: SectionBreakBlock = {
5844+
kind: 'sectionBreak',
5845+
id: 'sb2',
5846+
type: 'nextPage',
5847+
attrs: { source: 'sectPr', sectionIndex: 1 },
5848+
pageSize: { w: 600, h: 800 },
5849+
margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 },
5850+
};
5851+
5852+
const options: LayoutOptions = {
5853+
pageSize: { w: 600, h: 800 },
5854+
margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 },
5855+
alternateHeaders: true,
5856+
sectionMetadata: [{ sectionIndex: 0 }, { sectionIndex: 1, titlePg: true }],
5857+
headerContentHeights: {
5858+
first: 100, // section 2 title-page header
5859+
odd: 80,
5860+
even: 40,
5861+
},
5862+
};
5863+
5864+
const layout = layoutDocument(
5865+
[sb1, tallBlock('p1'), tallBlock('p2'), tallBlock('p3'), sb2, tallBlock('p4'), tallBlock('p5'), tallBlock('p6')],
5866+
[
5867+
{ kind: 'sectionBreak' },
5868+
tallMeasure,
5869+
tallMeasure,
5870+
tallMeasure,
5871+
{ kind: 'sectionBreak' },
5872+
tallMeasure,
5873+
tallMeasure,
5874+
tallMeasure,
5875+
],
5876+
options,
5877+
);
5878+
5879+
expect(layout.pages.length).toBeGreaterThanOrEqual(6);
5880+
5881+
// Page 4: section 2 first page + titlePg → 'first' (100px) → y = max(50, 30+100) = 130
5882+
const p4Fragment = layout.pages[3]?.fragments.find((f) => f.blockId === 'p4');
5883+
expect(p4Fragment).toBeDefined();
5884+
expect(p4Fragment!.y).toBeCloseTo(130, 0);
5885+
5886+
// Page 5: docPN=5, odd → 'odd' (80px) → y = max(50, 30+80) = 110
5887+
// If sectionPageNumber were used: sectionPN=2 → 'even' (40) → y = 70 (wrong)
5888+
const p5Fragment = layout.pages[4]?.fragments.find((f) => f.blockId === 'p5');
5889+
expect(p5Fragment).toBeDefined();
5890+
expect(p5Fragment!.y).toBeCloseTo(110, 0);
5891+
5892+
// Page 6: docPN=6, even → 'even' (40px) → y = max(50, 30+40) = 70
5893+
// If sectionPageNumber were used: sectionPN=3 → 'odd' (80) → y = 110 (wrong)
5894+
const p6Fragment = layout.pages[5]?.fragments.find((f) => f.blockId === 'p6');
5895+
expect(p6Fragment).toBeDefined();
5896+
expect(p6Fragment!.y).toBeCloseTo(70, 0);
58215897
});
58225898
});

packages/layout-engine/layout-engine/src/index.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,10 @@ export type LayoutOptions = {
533533
* Corresponds to the w:evenAndOddHeaders element in OOXML settings.xml.
534534
* When true, odd pages use the 'odd' variant and even pages use the 'even' variant.
535535
* When false or omitted, all pages use the 'default' variant.
536+
*
537+
* Must stay in sync with `getHeaderFooterTypeForSection` in
538+
* `layout-bridge/src/headerFooterUtils.ts` — both sides read this value
539+
* and must agree on variant selection.
536540
*/
537541
alternateHeaders?: boolean;
538542
};
@@ -676,26 +680,29 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
676680
/**
677681
* Determines the header/footer variant type for a given page based on section settings.
678682
*
683+
* Takes a params object because the two page-number fields have very similar
684+
* names and types — a positional call site is easy to get wrong.
685+
*
679686
* @param sectionPageNumber - The page number within the current section (1-indexed), used for titlePg
680687
* @param documentPageNumber - The absolute document page number (1-indexed), used for even/odd
681688
* @param titlePgEnabled - Whether the section has "different first page" enabled
682689
* @param alternateHeaders - Whether the document has odd/even differentiation enabled
683690
* @returns The variant type: 'first', 'even', 'odd', or 'default'
684691
*/
685-
const getVariantTypeForPage = (
686-
sectionPageNumber: number,
687-
documentPageNumber: number,
688-
titlePgEnabled: boolean,
689-
alternateHeaders: boolean,
690-
): 'default' | 'first' | 'even' | 'odd' => {
692+
const getVariantTypeForPage = (args: {
693+
sectionPageNumber: number;
694+
documentPageNumber: number;
695+
titlePgEnabled: boolean;
696+
alternateHeaders: boolean;
697+
}): 'default' | 'first' | 'even' | 'odd' => {
691698
// First page of section with titlePg enabled uses 'first' variant
692-
if (sectionPageNumber === 1 && titlePgEnabled) {
699+
if (args.sectionPageNumber === 1 && args.titlePgEnabled) {
693700
return 'first';
694701
}
695702
// Alternate headers: even/odd based on document page number, matching
696703
// the rendering side (getHeaderFooterTypeForSection in headerFooterUtils.ts)
697-
if (alternateHeaders) {
698-
return documentPageNumber % 2 === 0 ? 'even' : 'odd';
704+
if (args.alternateHeaders) {
705+
return args.documentPageNumber % 2 === 0 ? 'even' : 'odd';
699706
}
700707
return 'default';
701708
};
@@ -1308,7 +1315,12 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
13081315
const alternateHeaders = options.alternateHeaders ?? false;
13091316

13101317
// Determine which header/footer variant applies to this page
1311-
const variantType = getVariantTypeForPage(sectionPageNumber, newPageNumber, titlePgEnabled, alternateHeaders);
1318+
const variantType = getVariantTypeForPage({
1319+
sectionPageNumber,
1320+
documentPageNumber: newPageNumber,
1321+
titlePgEnabled,
1322+
alternateHeaders,
1323+
});
13121324

13131325
// Resolve header/footer refs for margin calculation using OOXML inheritance model.
13141326
// This must match the rendering logic in PresentationEditor to ensure margins

packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,16 +4238,10 @@ export class PresentationEditor extends EventEmitter {
42384238
? buildSemanticFootnoteBlocks(footnotesLayoutInput, this.#layoutOptions.semanticOptions?.footnotesMode)
42394239
: [];
42404240
const blocksForLayout = semanticFootnoteBlocks.length > 0 ? [...blocks, ...semanticFootnoteBlocks] : blocks;
4241-
const layoutOptions = {
4242-
...(!isSemanticFlow && footnotesLayoutInput
4241+
const layoutOptions =
4242+
!isSemanticFlow && footnotesLayoutInput
42434243
? { ...baseLayoutOptions, footnotes: footnotesLayoutInput }
4244-
: baseLayoutOptions),
4245-
...(isSemanticFlow
4246-
? {}
4247-
: {
4248-
alternateHeaders: Boolean((this.#editor as EditorWithConverter).converter?.pageStyles?.alternateHeaders),
4249-
}),
4250-
};
4244+
: baseLayoutOptions;
42514245
const previousBlocks = this.#layoutState.blocks;
42524246
const previousLayout = this.#layoutState.layout;
42534247
const previousMeasures = this.#layoutState.measures;
@@ -5416,12 +5410,17 @@ export class PresentationEditor extends EventEmitter {
54165410

54175411
this.#hiddenHost.style.width = `${pageSize.w}px`;
54185412

5413+
const alternateHeaders = Boolean(
5414+
(this.#editor as EditorWithConverter | undefined)?.converter?.pageStyles?.alternateHeaders,
5415+
);
5416+
54195417
return {
54205418
flowMode: 'paginated',
54215419
pageSize,
54225420
margins: resolvedMargins,
54235421
...(columns ? { columns } : {}),
54245422
sectionMetadata,
5423+
alternateHeaders,
54255424
};
54265425
}
54275426

0 commit comments

Comments
 (0)