Skip to content

Commit 884c7fb

Browse files
tupizzharbournick
andauthored
fix(pm-adapter): emit pending section break before TOC/docPartObj SDTs (SD-2557) (#2872)
* fix(pm-adapter): emit pending section break before TOC/docPartObj SDTs (SD-2557) When a Word document stores a `w:sectPr` on a paragraph immediately before a TOC (or other `docPartObj`) SDT, the section break was dropped. The TOC ended up rendering on the same page as the prior section's content instead of starting a new page as Word does. Root cause in pm-adapter: - `findParagraphsWithSectPr` (sections/analysis.ts) recurses into `index`, `bibliography`, and `tableOfAuthorities` to count their children as paragraphs, but NOT into `documentPartObject` / `tableOfContents`. - As a result, section ranges treat a TOC SDT as a single opaque unit — its children don't occupy paragraph indices. - When processing flow, `handleParagraphNode` / `handleIndexNode` / `handleBibliographyNode` / `handleTableOfAuthoritiesNode` each emit a pending section break before the paragraph whose index matches `nextSection.startParagraphIndex`. - `handleDocumentPartObjectNode` and `handleTableOfContentsNode` did NOT run this check, so the deferred break only fired on the next body paragraph AFTER the SDT. The SDT's content rendered in the PREVIOUS section, with no page break before it. Fix: - Add `emitPendingSectionBreakForParagraph(sectionState, nextBlockId, blocks, recordBlockKind)` helper in sections/breaks.ts that centralizes the "check, emit, advance" pattern. - Call the helper at the top of `handleDocumentPartObjectNode` and `handleTableOfContentsNode` — once per SDT. Since the SDT's children don't affect `currentParagraphIndex` (`findParagraphsWithSectPr` skips them), the check fires correctly at the SDT boundary: if the SDT sits at a section boundary, the nextPage break is emitted so the SDT renders on a new page. - No changes to section-range computation — counting stays consistent. Verified against both fixtures from the issue (Highstreet Proposal Sample, Heffernan Proposal Sample): cover stays on its own page, TOC starts on a new page, matching Word. Tests: - 3 new unit tests in document-part-object.test.ts covering: - Section break emitted at SDT boundary - No emission when SDT is not at a section boundary - No-op when sectionState is undefined - 1740 pm-adapter tests pass (up from 1737), 604 layout-engine tests pass. * fix(pm-adapter): recurse into docPartObj for section ranges + per-child emission (SD-2557) The initial fix only handled the sectPr BEFORE a TOC docPartObj (on the empty paragraph that precedes the SDT). It missed the SECOND sectPr that Word commonly stores on the trailing empty paragraph INSIDE the TOC SDT — which signals "TOC section ends here, next section starts on new page". Because `findParagraphsWithSectPr` did not recurse into `documentPartObject` or `tableOfContents`, that inner sectPr was never discovered, so no section-range boundary was built between the TOC and the following body content. SuperDoc rendered the TOC and the first body section stacked on the same page (just one page later than before the first fix). Complete fix: 1. `findParagraphsWithSectPr` (sections/analysis.ts) now recurses into `documentPartObject` and `tableOfContents` in addition to `index` / `bibliography` / `tableOfAuthorities`. This lets section-range analysis see sectPrs stored anywhere inside a TOC SDT. 2. `handleDocumentPartObjectNode` (non-TOC branch) emits the pending section break before each child paragraph and advances `currentParagraphIndex` — matching the pattern in `handleIndexNode`, `handleBibliographyNode`, and `handleTableOfAuthoritiesNode`. 3. `processTocChildren` (toc.ts) accepts `sectionState` via its context arg and performs the same per-child emit + increment dance as the paragraph handlers. This handles the TOC branch of `handleDocumentPartObjectNode` and the nested `tableOfContents` recursion path. With all three changes, the Highstreet fixture now renders exactly like Word: - Page 1: cover - Page 2: TOC alone - Page 3: ABOUT US body - Page 4: ON BEHALF OF HIGHSTREET - Page 5: WORKERS COMPENSATION Tests: - 4 new tests in document-part-object.test.ts (non-TOC emission, non-boundary no-op, undefined state, sectionState passthrough to processTocChildren) - 1741/1741 pm-adapter, 604/604 layout-engine, 11377/11377 super-editor * refactor(pm-adapter): unify TOC handling via processTocChildren (SD-2557) Collapse handleTableOfContentsNode into a delegation to processTocChildren so direct `tableOfContents` nodes share the same code path as the `documentPartObject` TOC gallery. Keeps the section-range counting contract from findParagraphsWithSectPr consistent across both paths: every TOC child paragraph advances sectionState.currentParagraphIndex, so deferred section breaks fire at the correct boundary. - Gate applyTocMetadata sdt fabrication on gallery — a direct tableOfContents PM node has no enclosing w:sdt, so we no longer invent a docPartObject SDT metadata entry on its entries. - Forward themeColors through processTocChildren to the paragraph converter; also pass it in from handleDocumentPartObjectNode. - Add regression tests for per-child index advancement, sdt gating, and themeColors pass-through. Addresses PR #2872 review feedback. --------- Co-authored-by: Nick Bernal <117235294+harbournick@users.noreply.github.com>
1 parent ffef210 commit 884c7fb

7 files changed

Lines changed: 453 additions & 57 deletions

File tree

packages/layout-engine/pm-adapter/src/sdt/document-part-object.test.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,5 +473,124 @@ describe('document-part-object', () => {
473473
expect(callArgs[1].tocInstruction).toBeUndefined();
474474
});
475475
});
476+
477+
// ==================== Pending section-break emission (SD-2557) ====================
478+
describe('pending section break at SDT boundary', () => {
479+
const sectionFixture = (startParagraphIndex: number) => ({
480+
ranges: [
481+
{
482+
sectionIndex: 0,
483+
startParagraphIndex: 0,
484+
endParagraphIndex: 0,
485+
sectPr: null,
486+
margins: null,
487+
headerRefs: {},
488+
footerRefs: {},
489+
type: 'nextPage',
490+
},
491+
{
492+
sectionIndex: 1,
493+
startParagraphIndex,
494+
endParagraphIndex: 10,
495+
sectPr: null,
496+
margins: null,
497+
headerRefs: {},
498+
footerRefs: {},
499+
type: 'nextPage',
500+
},
501+
],
502+
currentSectionIndex: 0,
503+
currentParagraphIndex: startParagraphIndex,
504+
});
505+
506+
// For the TOC branch, per-child emission now lives inside `processTocChildren`
507+
// (which is mocked in these tests). The non-TOC branch below exercises the
508+
// inline per-child emission path directly.
509+
it('emits a section break before a docPartObj non-TOC child at a section boundary', () => {
510+
// Repro for SD-2557 at the non-TOC path: same root cause — the handler
511+
// processes child paragraphs but previously skipped the section-break check.
512+
const node: PMNode = {
513+
type: 'documentPartObject',
514+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
515+
};
516+
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
517+
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
518+
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
519+
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });
520+
521+
// currentParagraphIndex === nextSection.startParagraphIndex → the first
522+
// child paragraph is the start of section 1.
523+
mockContext.sectionState = sectionFixture(3) as unknown as NodeHandlerContext['sectionState'];
524+
525+
handleDocumentPartObjectNode(node, mockContext);
526+
527+
const sectionBreak = mockContext.blocks.find((b) => b.kind === 'sectionBreak');
528+
expect(sectionBreak).toBeDefined();
529+
expect(mockContext.sectionState!.currentSectionIndex).toBe(1);
530+
// Counter must advance past the child paragraph so subsequent body
531+
// content sees the correct paragraph index.
532+
expect(mockContext.sectionState!.currentParagraphIndex).toBe(4);
533+
});
534+
535+
it('does not emit a section break when the child is not at a section boundary', () => {
536+
const node: PMNode = {
537+
type: 'documentPartObject',
538+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
539+
};
540+
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
541+
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
542+
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
543+
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });
544+
545+
// currentParagraphIndex (2) < startParagraphIndex (5): not at boundary.
546+
const state = sectionFixture(5);
547+
state.currentParagraphIndex = 2;
548+
mockContext.sectionState = state as unknown as NodeHandlerContext['sectionState'];
549+
550+
handleDocumentPartObjectNode(node, mockContext);
551+
552+
expect(mockContext.blocks.find((b) => b.kind === 'sectionBreak')).toBeUndefined();
553+
expect(mockContext.sectionState!.currentSectionIndex).toBe(0);
554+
// Counter still advances past the processed child.
555+
expect(mockContext.sectionState!.currentParagraphIndex).toBe(3);
556+
});
557+
558+
it('is a no-op when sectionState is undefined', () => {
559+
const node: PMNode = {
560+
type: 'documentPartObject',
561+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Page Number' }] }],
562+
};
563+
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Building Block Gallery');
564+
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('bb-1');
565+
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
566+
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });
567+
568+
mockContext.sectionState = undefined;
569+
570+
expect(() => handleDocumentPartObjectNode(node, mockContext)).not.toThrow();
571+
expect(mockContext.blocks.find((b) => b.kind === 'sectionBreak')).toBeUndefined();
572+
});
573+
574+
it('passes sectionState through to processTocChildren for TOC gallery', () => {
575+
const node: PMNode = {
576+
type: 'documentPartObject',
577+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'TOC Entry' }] }],
578+
};
579+
vi.mocked(metadataModule.getDocPartGallery).mockReturnValue('Table of Contents');
580+
vi.mocked(metadataModule.getDocPartObjectId).mockReturnValue('toc-1');
581+
vi.mocked(metadataModule.getNodeInstruction).mockReturnValue(undefined);
582+
vi.mocked(metadataModule.resolveNodeSdtMetadata).mockReturnValue({ type: 'docPartObject' });
583+
584+
const state = sectionFixture(3);
585+
mockContext.sectionState = state as unknown as NodeHandlerContext['sectionState'];
586+
587+
handleDocumentPartObjectNode(node, mockContext);
588+
589+
// processTocChildren is mocked; just verify it received sectionState
590+
// so the helper-inside-processTocChildren pattern can work end-to-end.
591+
const callArgs = vi.mocked(tocModule.processTocChildren).mock.calls[0];
592+
expect(callArgs[2]).toMatchObject({ sectionState: state });
593+
});
594+
});
476595
});
477596
});

packages/layout-engine/pm-adapter/src/sdt/document-part-object.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

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

@@ -14,6 +15,14 @@ import { processTocChildren } from './toc.js';
1415
* Processes TOC children for Table of Contents galleries.
1516
* For other gallery types (page numbers, etc.), processes child paragraphs normally.
1617
*
18+
* If a preceding paragraph carried a `w:sectPr` whose next section starts at
19+
* this SDT, emit the pending section break BEFORE processing children so the
20+
* SDT's paragraphs render on the new page (see SD-2557). `findParagraphsWithSectPr`
21+
* doesn't recurse into `documentPartObject`, so its child paragraphs don't bump
22+
* `currentParagraphIndex` — and without this call, the deferred break would only
23+
* fire on the next body paragraph AFTER the SDT, leaving e.g. a TOC on the
24+
* prior page with the cover content.
25+
*
1726
* @param node - Document part object node to process
1827
* @param context - Shared handler context
1928
*/
@@ -27,12 +36,14 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
2736
positions,
2837
bookmarks,
2938
hyperlinkConfig,
39+
sectionState,
3040
converters,
3141
converterContext,
3242
enableComments,
3343
trackedChangesConfig,
3444
themeColors,
3545
} = context;
46+
3647
const docPartGallery = getDocPartGallery(node);
3748
const docPartObjectId = getDocPartObjectId(node);
3849
const tocInstruction = getNodeInstruction(node);
@@ -50,15 +61,21 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
5061
hyperlinkConfig,
5162
enableComments,
5263
trackedChangesConfig,
64+
themeColors,
5365
converters,
5466
converterContext,
67+
sectionState,
5568
},
5669
{ blocks, recordBlockKind },
5770
);
5871
} else if (paragraphToFlowBlocks) {
59-
// For non-ToC gallery types (page numbers, etc.), process child paragraphs normally
72+
// For non-ToC gallery types (page numbers, etc.), process child paragraphs normally.
73+
// `findParagraphsWithSectPr` recurses into documentPartObject (SD-2557), so child
74+
// paragraph indices ARE counted — we must mirror that by emitting pending section
75+
// breaks and advancing currentParagraphIndex per child.
6076
for (const child of node.content) {
6177
if (child.type === 'paragraph') {
78+
emitPendingSectionBreakForParagraph({ sectionState, nextBlockId, blocks, recordBlockKind });
6279
const childBlocks = paragraphToFlowBlocks({
6380
para: child,
6481
nextBlockId,
@@ -75,6 +92,7 @@ export function handleDocumentPartObjectNode(node: PMNode, context: NodeHandlerC
7592
blocks.push(block);
7693
recordBlockKind?.(block.kind);
7794
}
95+
if (sectionState) sectionState.currentParagraphIndex++;
7896
}
7997
}
8098
}

0 commit comments

Comments
 (0)