Skip to content

Commit 09df6d1

Browse files
luccas-harbourluccas-harbour
committed
fix(layout): stop leaking converter fallback refs into section resolution
buildMultiSectionIdentifier previously merged the converter's legacy header/footer refs into section 0's resolution entry. This let a footerless first section inherit a converter-level default that belonged to a later section, painting a footer where the document declares none. Section-aware resolution now reads only per-section refs; converter fallbacks remain on the legacy identifier fields for legacy lookups but are no longer exposed through resolveEffectiveHeaderFooterRef. Guard HeaderFooterSessionManager so it only consults legacy refs when section resolution is unavailable, and skip building resolution sections for an empty identifier.
1 parent e856a49 commit 09df6d1

4 files changed

Lines changed: 295 additions & 55 deletions

File tree

‎packages/layout-engine/layout-bridge/src/headerFooterUtils.ts‎

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -182,50 +182,41 @@ export const defaultMultiSectionIdentifier = (): MultiSectionHeaderFooterIdentif
182182
sections: [],
183183
});
184184

185-
function mergeSection0FallbackRefs(
186-
sectionRefs: SectionHeaderFooterIds | undefined,
187-
fallbackRefs: SectionHeaderFooterIds,
188-
): SectionHeaderFooterIds {
189-
return {
190-
default: sectionRefs?.default ?? fallbackRefs.default,
191-
first: sectionRefs?.first ?? fallbackRefs.first,
192-
even: sectionRefs?.even ?? fallbackRefs.even,
193-
odd: sectionRefs?.odd ?? fallbackRefs.odd,
194-
};
195-
}
196-
197185
function refreshResolutionSections(identifier: MultiSectionHeaderFooterIdentifier): void {
186+
if (
187+
identifier.sectionCount === 0 &&
188+
identifier.sectionHeaderIds.size === 0 &&
189+
identifier.sectionFooterIds.size === 0 &&
190+
identifier.sectionTitlePg.size === 0
191+
) {
192+
identifier.sections = [];
193+
return;
194+
}
195+
198196
const maxIndex = Math.max(
199197
identifier.sectionCount - 1,
200198
...Array.from(identifier.sectionHeaderIds.keys()),
201199
...Array.from(identifier.sectionFooterIds.keys()),
202200
...Array.from(identifier.sectionTitlePg.keys()),
203-
0,
204201
);
205202

206203
const sections: HeaderFooterResolutionSection[] = [];
207204
for (let sectionIndex = 0; sectionIndex <= maxIndex; sectionIndex += 1) {
208205
sections.push({
209206
sectionIndex,
210-
titlePg: identifier.sectionTitlePg.has(sectionIndex)
211-
? identifier.sectionTitlePg.get(sectionIndex)
212-
: sectionIndex === 0
213-
? identifier.titlePg
214-
: false,
215-
headerRefs:
216-
sectionIndex === 0
217-
? mergeSection0FallbackRefs(identifier.sectionHeaderIds.get(sectionIndex), identifier.headerIds)
218-
: identifier.sectionHeaderIds.get(sectionIndex),
219-
footerRefs:
220-
sectionIndex === 0
221-
? mergeSection0FallbackRefs(identifier.sectionFooterIds.get(sectionIndex), identifier.footerIds)
222-
: identifier.sectionFooterIds.get(sectionIndex),
207+
titlePg: identifier.sectionTitlePg.get(sectionIndex) ?? false,
208+
headerRefs: identifier.sectionHeaderIds.get(sectionIndex),
209+
footerRefs: identifier.sectionFooterIds.get(sectionIndex),
223210
});
224211
}
225212

226213
identifier.sections = sections;
227214
}
228215

216+
function getSectionTitlePg(identifier: MultiSectionHeaderFooterIdentifier, sectionIndex: number): boolean {
217+
return identifier.sectionTitlePg.get(sectionIndex) ?? false;
218+
}
219+
229220
/**
230221
* Builds a multi-section header/footer identifier from section metadata.
231222
*
@@ -404,10 +395,9 @@ export function getHeaderFooterTypeForSection(
404395
const sectionPageNumber = options?.sectionPageNumber ?? pageNumber;
405396
const parityPageNumber = options?.parityPageNumber ?? pageNumber;
406397

407-
// Check titlePg for this specific section
408-
const sectionTitlePg = identifier.sectionTitlePg.has(sectionIndex)
409-
? identifier.sectionTitlePg.get(sectionIndex)!
410-
: identifier.titlePg;
398+
// Check titlePg for this specific section. Omitted section metadata means false;
399+
// legacy converter titlePg is only used by the non-section-aware path.
400+
const sectionTitlePg = getSectionTitlePg(identifier, sectionIndex);
411401
const variant = selectHeaderFooterVariantForPage({
412402
documentPageNumber: parityPageNumber,
413403
sectionPageNumber,
@@ -453,9 +443,7 @@ export function getHeaderFooterIdForPage(
453443
const sectionIndex = page.sectionIndex ?? 0;
454444
const sectionPageNumber = options?.sectionPageNumber ?? page.number;
455445
const parityPageNumber = options?.parityPageNumber ?? page.displayNumber ?? page.number;
456-
const sectionTitlePg = identifier.sectionTitlePg.has(sectionIndex)
457-
? identifier.sectionTitlePg.get(sectionIndex)!
458-
: identifier.titlePg;
446+
const sectionTitlePg = getSectionTitlePg(identifier, sectionIndex);
459447
const variantType = selectHeaderFooterVariantForPage({
460448
documentPageNumber: parityPageNumber,
461449
sectionPageNumber,
@@ -528,9 +516,7 @@ export function resolveHeaderFooterForPageAndSection(
528516
const sectionPageNumber = typeof firstPageInSection === 'number' ? pageNumber - firstPageInSection + 1 : pageNumber;
529517
const parityPageNumber = options?.parityPageNumber ?? page.displayNumber ?? pageNumber;
530518

531-
const sectionTitlePg = identifier.sectionTitlePg.has(sectionIndex)
532-
? identifier.sectionTitlePg.get(sectionIndex)!
533-
: identifier.titlePg;
519+
const sectionTitlePg = getSectionTitlePg(identifier, sectionIndex);
534520
const type = selectHeaderFooterVariantForPage({
535521
documentPageNumber: parityPageNumber,
536522
sectionPageNumber,

‎packages/layout-engine/layout-bridge/test/headerFooterUtils.test.ts‎

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ describe('headerFooterUtils', () => {
395395
expect(identifier.footerIds.even).toBe('converter-f-even');
396396
});
397397

398-
it('should expose converter fallbacks through section-aware resolution', () => {
398+
it('keeps converter fallbacks on legacy fields without exposing them through section-aware resolution', () => {
399399
const sectionMetadata: SectionMetadata[] = [
400400
{
401401
sectionIndex: 0,
@@ -409,17 +409,44 @@ describe('headerFooterUtils', () => {
409409
footerIds: { default: 'converter-f-default' },
410410
});
411411

412-
expect(getHeaderFooterTypeForSection(1, 0, identifier, { kind: 'header' })).toBe('default');
412+
expect(identifier.headerIds.default).toBe('converter-h-default');
413+
expect(identifier.footerIds.default).toBe('converter-f-default');
414+
expect(getHeaderFooterTypeForSection(1, 0, identifier, { kind: 'header' })).toBeNull();
413415
expect(
414416
getHeaderFooterIdForPage({ number: 1, fragments: [], sectionIndex: 0 }, identifier, { kind: 'header' }),
415-
).toBe('converter-h-default');
416-
expect(getHeaderFooterTypeForSection(1, 0, identifier, { kind: 'footer' })).toBe('default');
417+
).toBeNull();
418+
expect(getHeaderFooterTypeForSection(1, 0, identifier, { kind: 'footer' })).toBeNull();
419+
expect(
420+
getHeaderFooterIdForPage({ number: 1, fragments: [], sectionIndex: 0 }, identifier, { kind: 'footer' }),
421+
).toBeNull();
422+
});
423+
424+
it('does not apply a legacy converter default footer to a footerless first section', () => {
425+
const sectionMetadata: SectionMetadata[] = [
426+
{
427+
sectionIndex: 0,
428+
titlePg: false,
429+
},
430+
{
431+
sectionIndex: 15,
432+
footerRefs: { default: 'rId22' },
433+
titlePg: false,
434+
},
435+
];
436+
437+
const identifier = buildMultiSectionIdentifier(sectionMetadata, undefined, {
438+
footerIds: { default: 'rId22' },
439+
});
440+
417441
expect(
418442
getHeaderFooterIdForPage({ number: 1, fragments: [], sectionIndex: 0 }, identifier, { kind: 'footer' }),
419-
).toBe('converter-f-default');
443+
).toBeNull();
444+
expect(
445+
getHeaderFooterIdForPage({ number: 2, fragments: [], sectionIndex: 15 }, identifier, { kind: 'footer' }),
446+
).toBe('rId22');
420447
});
421448

422-
it('should preserve converter titlePg fallback for section 0 variant selection', () => {
449+
it('keeps converter titlePg fallback on legacy fields without exposing first refs through section-aware resolution', () => {
423450
const sectionMetadata: SectionMetadata[] = [
424451
{
425452
sectionIndex: 0,
@@ -431,9 +458,30 @@ describe('headerFooterUtils', () => {
431458
headerIds: { first: 'converter-h-first', titlePg: true },
432459
});
433460

461+
expect(identifier.titlePg).toBe(true);
462+
expect(identifier.headerIds.first).toBe('converter-h-first');
434463
expect(
435464
getHeaderFooterIdForPage({ number: 1, fragments: [], sectionIndex: 0 }, identifier, { kind: 'header' }),
436-
).toBe('converter-h-first');
465+
).toBeNull();
466+
});
467+
468+
it('does not apply legacy converter titlePg to section-aware variant selection when titlePg is omitted', () => {
469+
const sectionMetadata: SectionMetadata[] = [
470+
{
471+
sectionIndex: 0,
472+
headerRefs: { default: 'h0-default' },
473+
},
474+
];
475+
476+
const identifier = buildMultiSectionIdentifier(sectionMetadata, undefined, {
477+
headerIds: { first: 'legacy-first', titlePg: true },
478+
});
479+
480+
expect(identifier.titlePg).toBe(true);
481+
expect(getHeaderFooterTypeForSection(1, 0, identifier, { kind: 'header', sectionPageNumber: 1 })).toBe('default');
482+
expect(
483+
getHeaderFooterIdForPage({ number: 1, fragments: [], sectionIndex: 0 }, identifier, { kind: 'header' }),
484+
).toBe('h0-default');
437485
});
438486

439487
it('should NOT override existing section metadata with converter IDs', () => {
@@ -1049,7 +1097,7 @@ describe('headerFooterUtils', () => {
10491097
expect(resolved?.contentId).toBe('h0-default');
10501098
});
10511099

1052-
it('uses converter fallback refs when section metadata has no explicit refs', () => {
1100+
it('does not use converter fallback refs when section metadata has no explicit refs', () => {
10531101
const identifier = buildMultiSectionIdentifier([{ sectionIndex: 0 }], undefined, {
10541102
headerIds: { default: 'converter-default' },
10551103
});
@@ -1063,11 +1111,10 @@ describe('headerFooterUtils', () => {
10631111

10641112
const resolved = resolveHeaderFooterForPageAndSection(layout, 0, identifier, { kind: 'header' });
10651113

1066-
expect(resolved?.type).toBe('default');
1067-
expect(resolved?.contentId).toBe('converter-default');
1114+
expect(resolved).toBeNull();
10681115
});
10691116

1070-
it('uses converter fallback refs when only later sections define refs', () => {
1117+
it('does not use converter fallback refs when only later sections define refs', () => {
10711118
const identifier = buildMultiSectionIdentifier(
10721119
[{ sectionIndex: 0 }, { sectionIndex: 1, headerRefs: { default: 'section-1-default' } }],
10731120
undefined,
@@ -1083,11 +1130,10 @@ describe('headerFooterUtils', () => {
10831130

10841131
const resolved = resolveHeaderFooterForPageAndSection(layout, 0, identifier, { kind: 'header' });
10851132

1086-
expect(resolved?.type).toBe('default');
1087-
expect(resolved?.contentId).toBe('converter-default');
1133+
expect(resolved).toBeNull();
10881134
});
10891135

1090-
it('inherits converter fallback refs into later sections with partial refs', () => {
1136+
it('does not inherit converter fallback refs into later sections with partial refs', () => {
10911137
const identifier = buildMultiSectionIdentifier(
10921138
[{ sectionIndex: 0 }, { sectionIndex: 1, headerRefs: { even: 'section-1-even' } }],
10931139
undefined,
@@ -1106,8 +1152,7 @@ describe('headerFooterUtils', () => {
11061152

11071153
const resolved = resolveHeaderFooterForPageAndSection(layout, 1, identifier, { kind: 'header' });
11081154

1109-
expect(resolved?.type).toBe('default');
1110-
expect(resolved?.contentId).toBe('converter-default');
1155+
expect(resolved).toBeNull();
11111156
});
11121157

11131158
it('gets an inherited first content id from section 0 when section 1 omits it', () => {

‎packages/super-editor/src/editors/v1/core/presentation-editor/header-footer/HeaderFooterSessionManager.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ type SurfacePmEntry = {
7777
el: HTMLElement;
7878
};
7979

80+
function hasSectionRefsForKind(
81+
identifier: MultiSectionHeaderFooterIdentifier | null | undefined,
82+
kind: 'header' | 'footer',
83+
): identifier is MultiSectionHeaderFooterIdentifier {
84+
const refKey = kind === 'header' ? 'headerRefs' : 'footerRefs';
85+
return Boolean(identifier?.sections?.some((section) => section[refKey] !== undefined));
86+
}
87+
8088
// AIDEV-NOTE: compat-fallback - header/footer session interaction still keys
8189
// off `data-pm-*` (prep-002). DomPainter also stamps the parallel neutral
8290
// dataset (`data-layout-fragment-id` etc.) which a future v2 consumer can
@@ -2406,14 +2414,15 @@ export class HeaderFooterSessionManager {
24062414
sectionFirstPageNumbers.set(idx, p.number);
24072415
}
24082416
}
2417+
const hasSectionResolution = hasSectionRefsForKind(multiSectionId, kind);
24092418

24102419
return (pageNumber, pageMargins, page) => {
24112420
const sectionIndex = page?.sectionIndex ?? 0;
24122421
const firstPageInSection = sectionFirstPageNumbers.get(sectionIndex);
24132422
const sectionPageNumber =
24142423
typeof firstPageInSection === 'number' ? pageNumber - firstPageInSection + 1 : pageNumber;
24152424
const parityPageNumber = page?.displayNumber ?? pageNumber;
2416-
const headerFooterType = multiSectionId
2425+
const headerFooterType = hasSectionResolution
24172426
? getHeaderFooterTypeForSection(pageNumber, sectionIndex, multiSectionId, {
24182427
kind,
24192428
sectionPageNumber,
@@ -2425,7 +2434,7 @@ export class HeaderFooterSessionManager {
24252434
return null;
24262435
}
24272436

2428-
const effectiveRef = multiSectionId?.sections?.length
2437+
const effectiveRef = hasSectionResolution
24292438
? resolveEffectiveHeaderFooterRef({
24302439
sections: multiSectionId.sections,
24312440
sectionIndex,
@@ -2436,7 +2445,8 @@ export class HeaderFooterSessionManager {
24362445
const pageSectionRefs = kind === 'header' ? page?.sectionRefs?.headerRefs : page?.sectionRefs?.footerRefs;
24372446
const legacyRefs = kind === 'header' ? legacyIdentifier.headerIds : legacyIdentifier.footerIds;
24382447
const fallbackRef =
2439-
refForVariant(pageSectionRefs, headerFooterType) ?? refForVariant(legacyRefs, headerFooterType);
2448+
refForVariant(pageSectionRefs, headerFooterType) ??
2449+
(!hasSectionResolution ? refForVariant(legacyRefs, headerFooterType) : undefined);
24402450
const sectionRId = effectiveRef?.refId ?? fallbackRef?.refId;
24412451
const layoutVariantType = effectiveRef?.matchedVariant ?? fallbackRef?.matchedVariant ?? headerFooterType;
24422452

0 commit comments

Comments
 (0)