Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,5 +473,124 @@ describe('document-part-object', () => {
expect(callArgs[1].tocInstruction).toBeUndefined();
});
});

// ==================== Pending section-break emission (SD-2557) ====================
describe('pending section break at SDT boundary', () => {
const sectionFixture = (startParagraphIndex: number) => ({
ranges: [
{
sectionIndex: 0,
startParagraphIndex: 0,
endParagraphIndex: 0,
sectPr: null,
margins: null,
headerRefs: {},
footerRefs: {},
type: 'nextPage',
},
{
sectionIndex: 1,
startParagraphIndex,
endParagraphIndex: 10,
sectPr: null,
margins: null,
headerRefs: {},
footerRefs: {},
type: 'nextPage',
},
],
currentSectionIndex: 0,
currentParagraphIndex: startParagraphIndex,
});

// For the TOC branch, per-child emission now lives inside `processTocChildren`
// (which is mocked in these tests). The non-TOC branch below exercises the
// inline per-child emission path directly.
it('emits a section break before a docPartObj non-TOC child at a section boundary', () => {
// Repro for SD-2557 at the non-TOC path: same root cause — the handler
// processes child paragraphs but previously skipped the section-break check.
const node: PMNode = {
type: 'documentPartObject',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
};
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });

// currentParagraphIndex === nextSection.startParagraphIndex → the first
// child paragraph is the start of section 1.
mockContext.sectionState = sectionFixture(3) as unknown as NodeHandlerContext['sectionState'];

handleDocumentPartObjectNode(node, mockContext);

const sectionBreak = mockContext.blocks.find((b) => b.kind === 'sectionBreak');
expect(sectionBreak).toBeDefined();
expect(mockContext.sectionState!.currentSectionIndex).toBe(1);
// Counter must advance past the child paragraph so subsequent body
// content sees the correct paragraph index.
expect(mockContext.sectionState!.currentParagraphIndex).toBe(4);
});

it('does not emit a section break when the child is not at a section boundary', () => {
const node: PMNode = {
type: 'documentPartObject',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
};
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });

// currentParagraphIndex (2) < startParagraphIndex (5): not at boundary.
const state = sectionFixture(5);
state.currentParagraphIndex = 2;
mockContext.sectionState = state as unknown as NodeHandlerContext['sectionState'];

handleDocumentPartObjectNode(node, mockContext);

expect(mockContext.blocks.find((b) => b.kind === 'sectionBreak')).toBeUndefined();
expect(mockContext.sectionState!.currentSectionIndex).toBe(0);
// Counter still advances past the processed child.
expect(mockContext.sectionState!.currentParagraphIndex).toBe(3);
});

it('is a no-op when sectionState is undefined', () => {
const node: PMNode = {
type: 'documentPartObject',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
};
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });

mockContext.sectionState = undefined;

expect(() => handleDocumentPartObjectNode(node, mockContext)).not.toThrow();
expect(mockContext.blocks.find((b) => b.kind === 'sectionBreak')).toBeUndefined();
});

it('passes sectionState through to processTocChildren for TOC gallery', () => {
const node: PMNode = {
type: 'documentPartObject',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'TOC Entry' }] }],
};
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Table of Contents');
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('toc-1');
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });

const state = sectionFixture(3);
mockContext.sectionState = state as unknown as NodeHandlerContext['sectionState'];

handleDocumentPartObjectNode(node, mockContext);

// processTocChildren is mocked; just verify it received sectionState
// so the helper-inside-processTocChildren pattern can work end-to-end.
const callArgs = vi.mocked(tocModule.processTocChildren).mock.calls[0];
expect(callArgs[2]).toMatchObject({ sectionState: state });
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import type { PMNode, NodeHandlerContext } from '../types.js';
import { emitPendingSectionBreakForParagraph } from '../sections/index.js';
import { getDocPartGallery, getDocPartObjectId, getNodeInstruction, resolveNodeSdtMetadata } from './metadata.js';
import { processTocChildren } from './toc.js';

Expand All @@ -14,6 +15,14 @@ import { processTocChildren } from './toc.js';
* Processes TOC children for Table of Contents galleries.
* For other gallery types (page numbers, etc.), processes child paragraphs normally.
*
* If a preceding paragraph carried a `w:sectPr` whose next section starts at
* this SDT, emit the pending section break BEFORE processing children so the
* SDT's paragraphs render on the new page (see SD-2557). `findParagraphsWithSectPr`
* doesn't recurse into `documentPartObject`, so its child paragraphs don't bump
* `currentParagraphIndex` — and without this call, the deferred break would only
* fire on the next body paragraph AFTER the SDT, leaving e.g. a TOC on the
* prior page with the cover content.
*
* @param node - Document part object node to process
* @param context - Shared handler context
*/
Expand All @@ -27,12 +36,14 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
positions,
bookmarks,
hyperlinkConfig,
sectionState,
converters,
converterContext,
enableComments,
trackedChangesConfig,
themeColors,
} = context;

const docPartGallery = getDocPartGallery(node);
const docPartObjectId = getDocPartObjectId(node);
const tocInstruction = getNodeInstruction(node);
Expand All @@ -52,13 +63,18 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
trackedChangesConfig,
converters,
converterContext,
sectionState,
},
{ blocks, recordBlockKind },
);
} else if (paragraphToFlowBlocks) {
// For non-ToC gallery types (page numbers, etc.), process child paragraphs normally
// For non-ToC gallery types (page numbers, etc.), process child paragraphs normally.
// `findParagraphsWithSectPr` recurses into documentPartObject (SD-2557), so child
// paragraph indices ARE counted — we must mirror that by emitting pending section
// breaks and advancing currentParagraphIndex per child.
for (const child of node.content) {
if (child.type === 'paragraph') {
emitPendingSectionBreakForParagraph({ sectionState, nextBlockId, blocks, recordBlockKind });
const childBlocks = paragraphToFlowBlocks({
para: child,
nextBlockId,
Expand All @@ -75,6 +91,7 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
blocks.push(block);
recordBlockKind?.(block.kind);
}
if (sectionState) sectionState.currentParagraphIndex++;
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions packages/layout-engine/pm-adapter/src/sdt/toc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
ConverterContext,
ThemeColorPalette,
} from '../types.js';
import { emitPendingSectionBreakForParagraph } from '../sections/index.js';
import { applySdtMetadataToParagraphBlocks, getNodeInstruction } from './metadata.js';

/**
Expand Down Expand Up @@ -101,6 +102,7 @@ export function processTocChildren(
converters: NestedConverters;
converterContext: ConverterContext;
themeColors?: ThemeColorPalette;
sectionState?: NodeHandlerContext['sectionState'];
},
outputArrays: {
blocks: FlowBlock[];
Expand All @@ -113,6 +115,16 @@ export function processTocChildren(

children.forEach((child) => {
if (child.type === 'paragraph') {
// SD-2557: emit any pending section break before this child. `findParagraphsWithSectPr`
// recurses into documentPartObject, so TOC child paragraph indices are part of the
// section-range counting — advance the counter after processing to stay in sync.
emitPendingSectionBreakForParagraph({
sectionState: context.sectionState,
nextBlockId: context.nextBlockId,
blocks,
recordBlockKind,
});

// Direct paragraph child - convert and tag
const paragraphBlocks = paragraphConverter({
para: child,
Expand Down Expand Up @@ -140,6 +152,8 @@ export function processTocChildren(
blocks.push(block);
recordBlockKind?.(block.kind);
});

if (context.sectionState) context.sectionState.currentParagraphIndex++;
} else if (child.type === 'tableOfContents' && Array.isArray(child.content)) {
// Nested tableOfContents - recurse with potentially different instruction
const childInstruction = getNodeInstruction(child);
Expand Down Expand Up @@ -173,11 +187,16 @@ export function handleTableOfContentsNode(node: PMNode, context: NodeHandlerCont
trackedChangesConfig,
bookmarks,
hyperlinkConfig,
sectionState,
converters,
converterContext,
themeColors,
enableComments,
} = context;

// See handleDocumentPartObjectNode for rationale (SD-2557).
emitPendingSectionBreakForParagraph({ sectionState, nextBlockId, blocks, recordBlockKind });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep TOC section state aligned with counted child paragraphs

findParagraphsWithSectPr now counts tableOfContents child paragraphs (sections/analysis.ts), but this handler only emits a pending break once at node entry and never advances sectionState.currentParagraphIndex per TOC paragraph. When a document has a tableOfContents node before later section boundaries, the section cursor falls behind and later sectionBreak blocks are emitted at the wrong point (often only by the end-of-document fallback), which changes pagination compared with Word.

Useful? React with 👍 / 👎.


const tocInstruction = getNodeInstruction(node);
const paragraphToFlowBlocks = converters.paragraphToFlowBlocks;

Expand Down
18 changes: 17 additions & 1 deletion packages/layout-engine/pm-adapter/src/sections/analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,23 @@ export function findParagraphsWithSectPr(doc: PMNode): {
return;
}

if (node.type === 'index' || node.type === 'bibliography' || node.type === 'tableOfAuthorities') {
// Recurse into container node types that wrap body paragraphs. Children
// of these nodes are counted as paragraphs for section-range purposes and
// their handlers increment `currentParagraphIndex` + call the section-break
// emission helper per child.
//
// `documentPartObject` / `tableOfContents` are important for SD-2557:
// Word stores the closing sectPr of a TOC section on the trailing empty
// paragraph INSIDE the SDT. Without recursion, that sectPr is invisible to
// section-range analysis and the nextPage break between TOC and the next
// body section is silently dropped.
if (
node.type === 'index' ||
node.type === 'bibliography' ||
node.type === 'tableOfAuthorities' ||
node.type === 'documentPartObject' ||
node.type === 'tableOfContents'
) {
getNodeChildren(node).forEach(visitNode);
}
};
Expand Down
52 changes: 52 additions & 0 deletions packages/layout-engine/pm-adapter/src/sections/breaks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,55 @@ export function shouldRequirePageBoundary(current: SectionRange, next: SectionRa
export function hasIntrinsicBoundarySignals(_: SectionRange): boolean {
return false;
}

/**
* Minimal mutable sectionState shape used by section-break emission helpers.
* Kept local so callers can pass `NodeHandlerContext['sectionState']` directly.
*/
interface SectionStateMutable {
ranges: SectionRange[];
currentSectionIndex: number;
currentParagraphIndex: number;
}

/**
* Emit a pending section break before a paragraph if the current paragraph
* index matches the start of the next section.
*
* Centralizes the "check, emit, advance" pattern used by paragraph and SDT
* handlers. SDT handlers that process children as an opaque block (e.g.
* TOC/docPartObj where child paragraphs aren't counted by
* `findParagraphsWithSectPr`) should call this ONCE at the SDT boundary —
* if the SDT sits at a section boundary, this emits the break so the SDT's
* contents render on the new page.
*
* No-op when:
* - sectionState is undefined or has no ranges
* - currentParagraphIndex doesn't match the next section's startParagraphIndex
*
* Side effects (when emitted):
* - Pushes a sectionBreak block onto `blocks`
* - Invokes `recordBlockKind`
* - Increments `sectionState.currentSectionIndex`
*/
export function emitPendingSectionBreakForParagraph(args: {
sectionState: SectionStateMutable | undefined;
nextBlockId: BlockIdGenerator;
blocks: FlowBlock[];
recordBlockKind?: (kind: FlowBlock['kind']) => void;
}): void {
const { sectionState, nextBlockId, blocks, recordBlockKind } = args;
if (!sectionState || sectionState.ranges.length === 0) return;

const nextSection = sectionState.ranges[sectionState.currentSectionIndex + 1];
if (!nextSection || sectionState.currentParagraphIndex !== nextSection.startParagraphIndex) return;

const currentSection = sectionState.ranges[sectionState.currentSectionIndex];
const requiresPageBoundary =
shouldRequirePageBoundary(currentSection, nextSection) || hasIntrinsicBoundarySignals(nextSection);
const extraAttrs = requiresPageBoundary ? { requirePageBoundary: true } : undefined;
const sectionBreak = createSectionBreakBlock(nextSection, nextBlockId, extraAttrs);
blocks.push(sectionBreak);
recordBlockKind?.(sectionBreak.kind);
sectionState.currentSectionIndex++;
}
1 change: 1 addition & 0 deletions packages/layout-engine/pm-adapter/src/sections/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ export {
isSectionBreakBlock,
signaturesEqual,
shallowObjectEquals,
emitPendingSectionBreakForParagraph,
} from './breaks.js';
Loading