From f144519ba1f17350713d15a56b9eaaedbac5d7ed Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 3 Jun 2026 09:14:41 -0300 Subject: [PATCH 1/6] fix(layout-engine): balance explicit equal-width continuous columns balanceSectionOnPage skipped every section with equalWidth=false plus explicit widths, so continuous newspaper sections declared as with equal children (the common case) never balanced and rendered single-column. Narrow the skip to GENUINELY-unequal widths: explicit widths that are all equal now balance like implicit equal columns. Genuinely-unequal widths still fill column-by-column (Word parity, unchanged). (SD-2324) --- .../src/column-balancing.test.ts | 29 +++++++++++++++++++ .../layout-engine/src/column-balancing.ts | 22 ++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/column-balancing.test.ts b/packages/layout-engine/layout-engine/src/column-balancing.test.ts index 9b308ce7d1..3efa938364 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.test.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.test.ts @@ -441,6 +441,35 @@ describe('balanceSectionOnPage', () => { expect(result).toBeNull(); }); + it('balances explicit columns that declare EQUAL widths (equalWidth=0 with equal w:col widths)', () => { + // SD-2324: continuous newspaper sections commonly use `` + // with explicit `` children that are all EQUAL (e.g. 4×2340). The unequal-width + // skip must NOT catch these — they balance like implicit equal columns. Genuinely-unequal + // widths (the test above, [200,376]) are still skipped. + const top = 96; + const { fragments, measureMap, blockSectionMap } = buildSectionFixture(2, 6, 20, top); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, equalWidth: false, widths: [288, 288] }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + expect(result!.maxY).toBe(top + 60); + const col0 = fragments.filter((f) => f.x === 96).length; + const col1 = fragments.filter((f) => f.x === 96 + 288 + 48).length; + expect(col0).toBe(3); + expect(col1).toBe(3); + }); + it('only moves fragments of the target section when the page has mixed sections', () => { // Page has 3 fragments in section 1 (already positioned in col 0) and 6 in section 2. // Balancing section 2 must not touch section 1 fragments. diff --git a/packages/layout-engine/layout-engine/src/column-balancing.ts b/packages/layout-engine/layout-engine/src/column-balancing.ts index a367d3c823..113f41963a 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.ts @@ -670,15 +670,33 @@ export interface BalanceSectionOnPageArgs { * Guards (skip balancing when): * - Section has <= 1 column (nothing to balance) * - Section contains an explicit column break (author intent wins) - * - Section uses unequal column widths (Word doesn't rebalance these) + * - Section uses GENUINELY-unequal column widths (Word fills these column-by-column; + * explicit widths that are all equal still balance — SD-2324) * - No fragments on this page belong to the section */ +/** True when every explicit column width is equal within a sub-pixel tolerance. */ +function allColumnWidthsEqual(widths: number[]): boolean { + if (widths.length <= 1) return true; + const first = widths[0]; + return widths.every((w) => Math.abs(w - first) <= 0.5); +} + export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: number } | null { const { sectionColumns, sectionHasExplicitColumnBreak, sectionIndex, blockSectionMap, fragments } = args; if (sectionColumns.count <= 1) return null; if (sectionHasExplicitColumnBreak) return null; - if (sectionColumns.equalWidth === false && Array.isArray(sectionColumns.widths) && sectionColumns.widths.length > 0) { + // Genuinely-unequal explicit widths: Word fills these column-by-column rather than + // rebalancing, and the height-balancer measures each fragment at a single width so it + // can't reflow per column. Explicit widths that are all EQUAL (equalWidth="0" with every + // equal — the common continuous newspaper case) DO balance like implicit + // equal columns. (SD-2324) + if ( + sectionColumns.equalWidth === false && + Array.isArray(sectionColumns.widths) && + sectionColumns.widths.length > 0 && + !allColumnWidthsEqual(sectionColumns.widths) + ) { return null; } From 5b96267df21e3960638a52a9bc7bd9dc62b14099 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Wed, 3 Jun 2026 09:14:53 -0300 Subject: [PATCH 2/6] fix(layout-adapter): honor per-column w:space for unequal columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per ECMA-376 §17.6.4, when columns are not equal width (w:equalWidth=0) the section-level w:cols/@w:space is ignored and the inter-column gap comes from each . extractColumns used the section space, over-spacing explicit columns so their widths scaled down to fit and diverged from Word (e.g. the 2002 ISDA sections). Use the per-column w:space for unequal columns; equal-width columns keep the section space. Advances SD-2629 for the uniform-spacing case. (SD-2324) --- .../sections/extraction.test.ts | 40 +++++++++++++++++++ .../layout-adapter/sections/extraction.ts | 12 ++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts index 713a8a3746..c40f9234a0 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts @@ -288,6 +288,46 @@ describe('extraction', () => { }); }); + it('uses per-column w:space and ignores section w:space for unequal columns (ECMA-376 §17.6.4)', () => { + // SD-2324: the reported ISDA sections are + // with explicit children. Per §17.6.4, when columns are NOT equal width + // the section-level w:space is ignored — the inter-column gap is each column's own w:space. + // So the gap must be 0 (from the children), not 48px (from the 720-twip section space). + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '4', 'w:equalWidth': '0', 'w:space': '720' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2340', 'w:space': '0' } }, + { name: 'w:col', attributes: { 'w:w': '2340', 'w:space': '0' } }, + { name: 'w:col', attributes: { 'w:w': '2340', 'w:space': '0' } }, + { name: 'w:col', attributes: { 'w:w': '2340', 'w:space': '0' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ + count: 4, + gap: 0, + withSeparator: false, + widths: [156, 156, 156, 156], + equalWidth: false, + }); + }); + it('should handle section with only normalized margins and no sectPr elements', () => { const para: PMNode = { type: 'paragraph', diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts index 234d260aa5..f080155611 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts @@ -255,9 +255,15 @@ function extractColumns(elements: SectionElement[]): ColumnLayout | undefined { ? true : undefined; const columnChildren = Array.isArray(cols.elements) ? cols.elements.filter((child) => child?.name === 'w:col') : []; - const gapTwips = - cols.attributes['w:space'] ?? - columnChildren.find((child) => child?.attributes?.['w:space'] != null)?.attributes?.['w:space']; + // Per ECMA-376 §17.6.4, when columns are NOT equal width (`w:equalWidth="0"`), the + // section-level `w:cols/@w:space` is IGNORED — inter-column spacing comes from each + // ``'s own `w:space`. Only equal-width columns use the section space. Using the + // section space for explicit columns over-spaces them (forcing the widths to scale + // down to fit) and diverges from Word. (SD-2324; per-column-distinct spacing is SD-2629.) + const firstChildSpace = columnChildren.find((child) => child?.attributes?.['w:space'] != null)?.attributes?.[ + 'w:space' + ]; + const gapTwips = equalWidth === false ? (firstChildSpace ?? 0) : (cols.attributes['w:space'] ?? firstChildSpace); const gapInches = parseColumnGap(gapTwips as string | number | undefined); const widths = columnChildren .map((child) => Number(child.attributes?.['w:w'])) From 40d4aa2b885c3e197813c623d1d6b409b51e57c5 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 3 Jun 2026 20:08:06 -0300 Subject: [PATCH 3/6] fix(columns): equal-mode column correctness + explicit count cap (SD-2324) Equal-width sections (w:equalWidth="1" or omitted) now match Word: extraction drops child widths and takes the gap from the section w:space (default 720), and normalizeColumnLayout honours per-column widths only when w:equalWidth="0". For explicit columns (w:equalWidth="0"), cap the count to min(w:num, valid child-width count) at the source, so a w:num larger than the provided widths no longer creates surplus 1px phantom columns in the fill loop (which reads the raw count). A matching clamp in normalizeColumnLayout stays as a defensive net. --- .../contracts/src/column-layout.test.ts | 32 +++ .../contracts/src/column-layout.ts | 18 +- .../sections/extraction.test.ts | 212 ++++++++++++++++++ .../layout-adapter/sections/extraction.ts | 33 ++- 4 files changed, 283 insertions(+), 12 deletions(-) diff --git a/packages/layout-engine/contracts/src/column-layout.test.ts b/packages/layout-engine/contracts/src/column-layout.test.ts index f2107b9887..308db41899 100644 --- a/packages/layout-engine/contracts/src/column-layout.test.ts +++ b/packages/layout-engine/contracts/src/column-layout.test.ts @@ -95,6 +95,38 @@ describe('normalizeColumnLayout', () => { }); }); + it('ignores widths when equalWidth is omitted and divides evenly (SD-2324: omitted = equal mode)', () => { + // Omitted equalWidth is equal mode in Word; any widths present are not authoritative. + expect(normalizeColumnLayout({ count: 2, gap: 24, widths: [100, 200] }, 624)).toEqual({ + count: 2, + gap: 24, + widths: [300, 300], + width: 300, + }); + }); + + it('ignores widths when equalWidth is true and divides evenly (SD-2324)', () => { + expect(normalizeColumnLayout({ count: 2, gap: 24, widths: [100, 200], equalWidth: true }, 624)).toEqual({ + count: 2, + gap: 24, + widths: [300, 300], + equalWidth: true, + width: 300, + }); + }); + + it('clamps count to the explicit-widths length when w:num exceeds it (SD-2324 F8)', () => { + // w:num="4" with only two explicit widths: the surplus columns have no width and must not + // be synthesized as ~0px slivers (the F8 phantom-column bug). Clamp to the two real columns. + expect(normalizeColumnLayout({ count: 4, gap: 48, widths: [192, 384], equalWidth: false }, 624)).toEqual({ + count: 2, + gap: 48, + widths: [192, 384], + equalWidth: false, + width: 384, + }); + }); + it('falls back to a single column when there is no usable content width', () => { expect(normalizeColumnLayout({ count: 3, gap: 24 }, 0, 0.01)).toEqual({ count: 1, diff --git a/packages/layout-engine/contracts/src/column-layout.ts b/packages/layout-engine/contracts/src/column-layout.ts index 7dc794c592..9369e32ed4 100644 --- a/packages/layout-engine/contracts/src/column-layout.ts +++ b/packages/layout-engine/contracts/src/column-layout.ts @@ -30,14 +30,24 @@ export function normalizeColumnLayout( epsilon = 0.0001, ): NormalizedColumnLayout { const rawCount = input && Number.isFinite(input.count) ? Math.floor(input.count) : 1; - const count = Math.max(1, rawCount || 1); + let count = Math.max(1, rawCount || 1); const gap = Math.max(0, input?.gap ?? 0); - const totalGap = gap * (count - 1); - const availableWidth = contentWidth - totalGap; + // Honor per-column widths ONLY in explicit mode (`equalWidth === false`). In equal mode + // (true or omitted) Word ignores child widths and divides the content area evenly, so any + // widths that reach here are not authoritative and must not drive geometry. (SD-2324) const explicitWidths = - Array.isArray(input?.widths) && input.widths.length > 0 + input?.equalWidth === false && Array.isArray(input?.widths) && input.widths.length > 0 ? input.widths.filter((width) => typeof width === 'number' && Number.isFinite(width) && width > 0) : []; + // Explicit columns are defined by their widths. When the section declares more + // columns than it supplies widths (e.g. w:num="4" with two ), the surplus columns + // have no width and previously padded to ~0px, rendering as 1px slivers of vertical text + // (SD-2324 F8). Clamp the count to the widths actually provided so every column renders. + if (explicitWidths.length > 0 && explicitWidths.length < count) { + count = explicitWidths.length; + } + const totalGap = gap * (count - 1); + const availableWidth = contentWidth - totalGap; let widths = explicitWidths.length > 0 diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts index c40f9234a0..436f63cb26 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts @@ -328,6 +328,218 @@ describe('extraction', () => { }); }); + it('drops child widths and uses the section gap when w:equalWidth="1" (equal mode, Word ignores children)', () => { + // SD-2324: Word treats w:equalWidth="1" as equal mode regardless of any children. + // It ignores child widths/spaces, derives equal columns from w:num, and the gap from w:cols/@w:space. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:equalWidth': '1', 'w:space': '720' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880' } }, + { name: 'w:col', attributes: { 'w:w': '5760' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + // No widths emitted; gap from the 720-twip section space (48px). Word equalizes such columns. + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, + withSeparator: false, + equalWidth: true, + }); + }); + + it('drops child widths when w:equalWidth is omitted (omitted defaults to equal mode, like Word)', () => { + // SD-2324: an omitted w:equalWidth is equal mode in Word (verified: EvenlySpaced=true). Child + // values must NOT leak through as explicit widths. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880', 'w:space': '720' } }, + { name: 'w:col', attributes: { 'w:w': '5760' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + // No widths, no equalWidth field; gap from the section space (48px). + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, + withSeparator: false, + }); + }); + + it('ignores child w:space and defaults the gap to 720 twips in equal mode (SD-2324 gap-half)', () => { + // SD-2324: in equal mode the gap comes from w:cols/@w:space only. With the section space omitted, + // it defaults to 720 twips (48px) even though the children declare w:space="0". Consulting the + // child space here (the pre-fix behavior) would wrongly yield a 0px gap. Verified against Word. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880', 'w:space': '0' } }, + { name: 'w:col', attributes: { 'w:w': '2880', 'w:space': '0' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, // 720-twip default, NOT the child w:space of 0 + withSeparator: false, + }); + }); + + it('keeps explicit child widths with a 0 gap when w:equalWidth="0" and no child w:space (SD-2324 F5)', () => { + // Explicit mode is unchanged by the equal-mode fix: child widths are honored, and an absent child + // w:space yields a 0 gap (CT_Column/@space default 0), not the 720-twip section default. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:equalWidth': '0' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '4680' } }, + { name: 'w:col', attributes: { 'w:w': '4680' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 0, + withSeparator: false, + widths: [312, 312], + equalWidth: false, + }); + }); + + it('resolves explicit-mode count as min(w:num default 1, valid child-width count); omitted num stays 1 (SD-2324, Word-verified)', () => { + // Word caps the explicit column count to the children actually provided. With w:num + // omitted (default 1) and 3 children, Word renders 1 column (verified Count=1), not 3. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:equalWidth': '0' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880' } }, + { name: 'w:col', attributes: { 'w:w': '2880' } }, + { name: 'w:col', attributes: { 'w:w': '2880' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + // min(1, 3) -> count is 1 (NOT 3 from the children). + expect(result?.columnsPx?.count).toBe(1); + expect(result?.columnsPx?.equalWidth).toBe(false); + }); + + it('caps explicit count to the valid child-width count when w:num exceeds it (SD-2324 F8)', () => { + // w:num="4" with only two renders 2 columns in Word (verified), not 4. Capping the + // count at the source keeps the fill loop (which reads the raw count) from creating surplus + // 1px phantom columns. min(4, 2) -> 2. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '4', 'w:equalWidth': '0' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880', 'w:space': '720' } }, + { name: 'w:col', attributes: { 'w:w': '5760' } }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, + withSeparator: false, + widths: [192, 384], + equalWidth: false, + }); + }); + it('should handle section with only normalized margins and no sectPr elements', () => { const para: PMNode = { type: 'paragraph', diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts index f080155611..968551af08 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.ts @@ -245,7 +245,7 @@ function extractColumns(elements: SectionElement[]): ColumnLayout | undefined { const cols = elements.find((el) => el?.name === 'w:cols'); if (!cols?.attributes) return undefined; - const count = parseColumnCount(cols.attributes['w:num'] as string | number | undefined); + let count = parseColumnCount(cols.attributes['w:num'] as string | number | undefined); const withSeparator = parseColumnSeparator(cols.attributes['w:sep'] as string | number | undefined); const equalWidthRaw = cols.attributes['w:equalWidth']; const equalWidth = @@ -255,26 +255,43 @@ function extractColumns(elements: SectionElement[]): ColumnLayout | undefined { ? true : undefined; const columnChildren = Array.isArray(cols.elements) ? cols.elements.filter((child) => child?.name === 'w:col') : []; - // Per ECMA-376 §17.6.4, when columns are NOT equal width (`w:equalWidth="0"`), the - // section-level `w:cols/@w:space` is IGNORED — inter-column spacing comes from each - // ``'s own `w:space`. Only equal-width columns use the section space. Using the - // section space for explicit columns over-spaces them (forcing the widths to scale - // down to fit) and diverges from Word. (SD-2324; per-column-distinct spacing is SD-2629.) + // ECMA-376 §17.6.4 column mode, validated against Word (MS Word 16 oracle): + // Explicit mode (`w:equalWidth="0"`): widths and inter-column spacing come from the child + // `` elements (`w:w` + `w:space`, default 0); the section `w:cols/@w:space` is + // ignored. (Per-column distinct spacing is SD-2629; today the first child's space is + // projected as the single gap.) + // Equal mode (`w:equalWidth="1"` or omitted): Word ignores all child `` data. The + // gap comes from `w:cols/@w:space` (default 720); a child `w:space` is NOT consulted, and + // child widths are dropped so the columns divide evenly. Count comes from `w:num` + // (default 1) in equal mode, and is capped to the valid child-width count in explicit + // mode (Word renders min(num, count of with a usable w:w)). (SD-2324) + const isExplicit = equalWidth === false; const firstChildSpace = columnChildren.find((child) => child?.attributes?.['w:space'] != null)?.attributes?.[ 'w:space' ]; - const gapTwips = equalWidth === false ? (firstChildSpace ?? 0) : (cols.attributes['w:space'] ?? firstChildSpace); + const gapTwips = isExplicit ? (firstChildSpace ?? 0) : cols.attributes['w:space']; const gapInches = parseColumnGap(gapTwips as string | number | undefined); const widths = columnChildren .map((child) => Number(child.attributes?.['w:w'])) .filter((widthTwips) => Number.isFinite(widthTwips) && widthTwips > 0) .map((widthTwips) => (widthTwips / 1440) * PX_PER_INCH); + // Explicit mode: w:num is capped to the valid child-width count (widths.length), i.e. the + // number of that supplied a usable w:w. Word renders min(num, that count) (e.g. + // w:num="4" with two => 2 columns, verified vs Word). This is the authoritative + // count both the fill loop and width math read; the matching clamp in normalizeColumnLayout + // is a defensive net for any other producer. (SD-2324 F8) + if (isExplicit && widths.length > 0) { + count = Math.min(count, widths.length); + } + const result: ColumnLayout = { count, gap: gapInches * PX_PER_INCH, withSeparator, - ...(widths.length > 0 ? { widths } : {}), + // Only explicit columns carry per-column widths; equal mode divides evenly (Word ignores + // child `w:w` when equalWidth is "1" or omitted). + ...(isExplicit && widths.length > 0 ? { widths } : {}), ...(equalWidth !== undefined ? { equalWidth } : {}), }; From 9ad7abddd5bcb34e20e2688678e885ac82535d68 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 3 Jun 2026 21:28:35 -0300 Subject: [PATCH 4/6] test(columns): cover valid-width count cap, equal-mode count, and absent w:cols (SD-2324) Adds three extraction unit tests for the landed column fix: count caps to the valid child-width count (four but two usable w:w -> 2), equal mode takes the count from w:num (count 3, no children), and a section without yields no columnsPx. --- .../sections/extraction.test.ts | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts index 436f63cb26..67e5d66319 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/sections/extraction.test.ts @@ -540,6 +540,86 @@ describe('extraction', () => { }); }); + it('caps to the valid child-width count, ignoring with no usable w:w (SD-2324)', () => { + // Four but only two carry a usable w:w; the count caps to those two (widths.length), + // not the raw four children. min(4, 2) -> 2. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '4', 'w:equalWidth': '0' }, + elements: [ + { name: 'w:col', attributes: { 'w:w': '2880' } }, + { name: 'w:col', attributes: { 'w:w': '5760' } }, + { name: 'w:col', attributes: { 'w:w': '0' } }, + { name: 'w:col', attributes: {} }, + ], + }, + ], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 0, + withSeparator: false, + widths: [192, 384], + equalWidth: false, + }); + }); + + it('takes the count from w:num in equal mode (count 3, no children) (SD-2324)', () => { + // Equal mode (omitted equalWidth) takes the count straight from w:num and the gap from the + // section w:space (720 twips -> 48px); no per-column widths are emitted. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [{ name: 'w:cols', attributes: { 'w:num': '3', 'w:space': '720' } }], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ count: 3, gap: 48, withSeparator: false }); + }); + + it('returns no columnsPx when the section has no element (SD-2324)', () => { + // A sectPr without must not synthesize a column layout. + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [{ name: 'w:pgSz', attributes: { 'w:w': '12240', 'w:h': '15840' } }], + }, + }, + }, + }; + + const result = extractSectionData(para); + + expect(result).not.toBeNull(); + expect(result?.columnsPx).toBeUndefined(); + }); + it('should handle section with only normalized margins and no sectPr elements', () => { const para: PMNode = { type: 'paragraph', From 85c8ff58c6c7f0de958467afb1b7e253dc4002db Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Thu, 4 Jun 2026 16:02:57 -0300 Subject: [PATCH 5/6] fix(layout-engine): balance continuous multi-column sections like Word A two-column section ending at a continuous section break rendered with lumpy columns (SD-3359, IT-1150): on the repro NDA the left column ran 169px deeper than the right, and the following single-column content started below the unbalanced overhang. Three defects, all in the balancing path: - balanceSectionOnPage fed the balancer atomic fragments (canBreak: false, no lineHeights), so a paragraph straddling the column boundary could not split. The balancer already computes line-level break points (blockBreakPoints) with widow/orphan control, but no caller consumed them. Paragraph fragments now opt in with their per-line heights, and a chosen break is applied as fragment surgery: the first half keeps the leading lines, a cloned second half carries the remaining lines to the top of the next column, the same fromLine/toLine and continuation model pagination uses. Paragraphs with w:keepLines (ECMA-376 17.3.1.14) and sectPr marker paragraphs stay atomic. - The binary search floored its target at the tallest block's full height. For a breakable paragraph the indivisible chunk is its tallest line, so the search could never reach the truly balanced height and packed the overflow lines into column 0. - The post-layout gate keyed "mid-doc continuous" off the section's own begin type (its sectPr w:type, 17.6.22) instead of the type of the break that ends it, which is the NEXT section's begin type. A 2-col section that merely started continuous balanced even when ended by a nextPage break; per the 17.18.77 note only a continuous break balances the previous section. Verified against the IT-1150 repro (169px -> 14px imbalance, trailing paragraph tucked directly below the balanced region) and eleven spec-derived document mutations (3 columns, explicit equal and unequal w:col, keepLines, explicit column break, nextPage/evenPage/nextColumn end breaks, w:sep, implicit and explicit body sectPr): all conform to ECMA-376 and the documented Word behaviors (sd-1655, sd-1480, mixed-columns-tabs-tnr). Adds 4 unit tests and 2 layoutDocument integration tests. --- .../src/column-balancing.test.ts | 150 ++++++++++++++++++ .../layout-engine/src/column-balancing.ts | 92 +++++++++-- .../layout-engine/src/index.test.ts | 86 +++++++++- .../layout-engine/layout-engine/src/index.ts | 40 ++++- 4 files changed, 349 insertions(+), 19 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/column-balancing.test.ts b/packages/layout-engine/layout-engine/src/column-balancing.test.ts index 3efa938364..f0d04c3538 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.test.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.test.ts @@ -525,4 +525,154 @@ describe('balanceSectionOnPage', () => { expect(result).toBeNull(); }); + + // SD-3359: Word balances a continuous multi-column section by flowing content + // line-by-line — a paragraph that straddles the column boundary SPLITS at a line + // boundary (the IT-1150 complaint). Atomic per-fragment assignment leaves the + // columns lumpy whenever one fragment is large relative to the section. + describe('paragraph line splitting across columns (SD-3359)', () => { + type SplitFragment = TestFragment & { + fromLine?: number; + toLine?: number; + continuesFromPrev?: boolean; + continuesOnNext?: boolean; + }; + const LINE = 20; + const TOP = 96; + const COL1_X = 96 + 288 + 48; + + /** A (5 lines) + B (3 lines) + C (14 lines): atomic best is 160 | 280 (120px lumpy); + * line-balanced is 220 | 220 with C split across the boundary. */ + function straddleFixture(cLines = 14): { + fragments: SplitFragment[]; + measureMap: Map }>; + blockSectionMap: Map; + } { + const mk = (id: string, y: number): SplitFragment => ({ + blockId: id, + x: 96, + y, + width: 624, + kind: 'para', + }); + const fragments = [mk('A', TOP), mk('B', TOP + 100), mk('C', TOP + 160)]; + const measureMap = new Map }>([ + ['A', createMeasure('paragraph', Array(5).fill(LINE))], + ['B', createMeasure('paragraph', Array(3).fill(LINE))], + ['C', createMeasure('paragraph', Array(cLines).fill(LINE))], + ]); + const blockSectionMap = new Map([ + ['A', 1], + ['B', 1], + ['C', 1], + ]); + return { fragments, measureMap, blockSectionMap }; + } + + const balance = ( + fragments: SplitFragment[], + measureMap: Map }>, + blockSectionMap: Map, + extra: Record = {}, + ) => + balanceSectionOnPage({ + fragments, + sectionIndex: 1, + sectionColumns: { count: 2, gap: 48, width: 288 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: TOP, + columnWidth: 288, + availableHeight: 720, + measureMap, + ...extra, + }); + + it('splits a straddling paragraph at a line boundary so columns balance', () => { + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + + const result = balance(fragments, measureMap, blockSectionMap); + + expect(result).not.toBeNull(); + // C was split into two fragments. + const cFrags = fragments.filter((f) => f.blockId === 'C') as SplitFragment[]; + expect(cFrags.length).toBe(2); + const [c1, c2] = cFrags.sort((a, b) => (a.fromLine ?? 0) - (b.fromLine ?? 0)); + // The halves partition C's lines contiguously. + expect(c1.toLine).toBe(c2.fromLine!); + expect(c2.toLine).toBe(14); + // First half continues in col 0 below A+B; second half tops col 1. + expect(c1.x).toBe(96); + expect(c2.x).toBe(COL1_X); + expect(c2.y).toBe(TOP); + expect(c1.continuesOnNext).toBe(true); + expect(c2.continuesFromPrev).toBe(true); + // Column bottoms balance within one line height (vs 120px atomic lumpiness). + const bottom = (f: SplitFragment): number => { + const from = f.fromLine ?? 0; + const to = f.toLine ?? measureMap.get(f.blockId)!.lines.length; + return f.y + (to - from) * LINE; + }; + const col0Bottom = Math.max(...fragments.filter((f) => f.x === 96).map(bottom)); + const col1Bottom = Math.max(...fragments.filter((f) => f.x === COL1_X).map(bottom)); + expect(Math.abs(col0Bottom - col1Bottom)).toBeLessThanOrEqual(LINE); + // The balanced bottom beats the atomic assignment (TOP + 280). + expect(result!.maxY).toBeLessThan(TOP + 280); + expect(result!.maxY).toBe(Math.max(col0Bottom, col1Bottom)); + }); + + it('does not split a paragraph with keepLines (author intent wins)', () => { + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + + const result = balance(fragments, measureMap, blockSectionMap, { + keepLinesBlockIds: new Set(['C']), + }); + + expect(result).not.toBeNull(); + // C stays whole — no extra fragment, no partial line range. + expect(fragments.filter((f) => f.blockId === 'C').length).toBe(1); + const c = fragments.find((f) => f.blockId === 'C')! as SplitFragment; + expect(c.fromLine ?? 0).toBe(0); + expect(c.toLine ?? 14).toBe(14); + }); + + it('balances a single tall paragraph alone in the section by splitting it', () => { + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + const only = [{ ...fragments[2], y: TOP }]; // C alone (14 lines = 280px) + + const result = balance(only, measureMap, blockSectionMap); + + // Previously skipped (single atomic block can't distribute); a breakable + // paragraph CAN balance — Word splits it across the columns. + expect(result).not.toBeNull(); + expect(only.length).toBe(2); + const [c1, c2] = (only as SplitFragment[]).sort((a, b) => (a.fromLine ?? 0) - (b.fromLine ?? 0)); + expect(c1.toLine).toBe(c2.fromLine!); + expect(c2.toLine).toBe(14); + expect(result!.maxY).toBeLessThan(TOP + 280); + }); + + it('offsets the split by the fragment fromLine when pagination already split the paragraph', () => { + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + // C is the tail of a 16-line paragraph: this page renders lines [2, 16). + measureMap.set('C', createMeasure('paragraph', Array(16).fill(LINE))); + const c = fragments[2]; + c.fromLine = 2; + c.toLine = 16; + + const result = balance(fragments, measureMap, blockSectionMap); + + expect(result).not.toBeNull(); + const cFrags = (fragments.filter((f) => f.blockId === 'C') as SplitFragment[]).sort( + (a, b) => (a.fromLine ?? 0) - (b.fromLine ?? 0), + ); + expect(cFrags.length).toBe(2); + const [c1, c2] = cFrags; + expect(c1.fromLine).toBe(2); + expect(c1.toLine).toBe(c2.fromLine!); + expect(c2.toLine).toBe(16); + expect(c2.fromLine!).toBeGreaterThan(2); + }); + }); }); diff --git a/packages/layout-engine/layout-engine/src/column-balancing.ts b/packages/layout-engine/layout-engine/src/column-balancing.ts index 113f41963a..e3c3f779d0 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.ts @@ -156,9 +156,18 @@ export function calculateBalancedColumnHeight( }; } - // Calculate total content height and block-height extremes + // Calculate total content height and block-height extremes. A column can + // never be shorter than its tallest INDIVISIBLE chunk: the full height for + // an unbreakable block, but only the tallest LINE for a breakable paragraph + // (SD-3359 — flooring at a breakable paragraph's full height pinned the + // search above the balanced height and packed the overflow lines into the + // first column instead of splitting evenly). const totalHeight = ctx.contentBlocks.reduce((sum, b) => sum + b.measuredHeight, 0); - const maxBlockHeight = ctx.contentBlocks.reduce((m, b) => Math.max(m, b.measuredHeight), 0); + const maxBlockHeight = ctx.contentBlocks.reduce((m, b) => { + const indivisible = + b.canBreak && b.lineHeights && b.lineHeights.length > 1 ? Math.max(...b.lineHeights) : b.measuredHeight; + return Math.max(m, indivisible); + }, 0); // Early exit: content is very small, no need to balance if (totalHeight < config.minColumnHeight * ctx.columnCount) { @@ -658,6 +667,12 @@ export interface BalanceSectionOnPageArgs { * Optional; when omitted no fragment is treated as a marker. */ sectPrMarkerBlockIds?: Set; + /** + * Block IDs of paragraphs with `w:keepLines` — the author asked Word not to + * split these, so they stay atomic during balancing. Optional; when omitted + * every multi-line paragraph is splittable. (SD-3359) + */ + keepLinesBlockIds?: Set; } /** @@ -777,13 +792,36 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu // Use `getBalancingHeight` so empty sectPr-marker paragraphs contribute 0 // to their column's cursor — matching Word's behavior of not rendering a // blank line for such markers. - const contentBlocks: BalancingBlock[] = ordered.map((f, i) => ({ - blockId: `${f.blockId}#${i}`, - measuredHeight: getBalancingHeight(f, args.measureMap, args.sectPrMarkerBlockIds), - canBreak: false, - keepWithNext: false, - keepTogether: true, - })); + // + // SD-3359: multi-line paragraphs additionally expose their per-line heights so + // the balancer can SPLIT a paragraph that straddles the column boundary (Word + // flows content line-by-line when balancing a continuous section, ECMA-376 + // §17.18.77 — atomic assignment leaves the columns lumpy whenever one + // paragraph is large relative to the section). sectPr markers, `w:keepLines` + // paragraphs, non-paragraph fragments, and single-line paragraphs stay atomic. + const lineHeightsFor = (f: BalancingFragment): number[] | undefined => { + if (f.kind !== 'para') return undefined; + if (args.sectPrMarkerBlockIds?.has(f.blockId)) return undefined; + if (args.keepLinesBlockIds?.has(f.blockId)) return undefined; + const measure = args.measureMap.get(f.blockId); + if (!measure || measure.kind !== 'paragraph' || !Array.isArray(measure.lines)) return undefined; + const fromLine = f.fromLine ?? 0; + const toLine = f.toLine ?? measure.lines.length; + if (toLine - fromLine <= 1) return undefined; + return measure.lines.slice(fromLine, toLine).map((l) => l.lineHeight); + }; + + const contentBlocks: BalancingBlock[] = ordered.map((f, i) => { + const lineHeights = lineHeightsFor(f); + return { + blockId: `${f.blockId}#${i}`, + measuredHeight: getBalancingHeight(f, args.measureMap, args.sectPrMarkerBlockIds), + canBreak: lineHeights !== undefined, + keepWithNext: false, + keepTogether: lineHeights === undefined, + lineHeights, + }; + }); if ( shouldSkipBalancing({ @@ -814,6 +852,42 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu f.x = columnX(col); f.y = colCursors[col]; f.width = columnWidth; + // SD-3359: apply a line-boundary split chosen by the balancer. The first + // half keeps the leading lines in this column; a cloned second half carries + // the remaining lines to the top of the next column — the same + // fromLine/toLine + continuation-flag surgery pagination uses when a + // paragraph splits across pages. The simulation assigns the block to the + // column of its FIRST half and flows the remainder into the next column, + // so the cursors advance by the split heights it computed. + const bp = result.blockBreakPoints?.get(block.blockId); + if (bp && bp.heightAfterBreak > 0 && col < columnCount - 1) { + const fromLine = f.fromLine ?? 0; + const splitLine = fromLine + bp.breakAfterLine + 1; + const measureLineCount = args.measureMap.get(f.blockId)?.lines?.length ?? splitLine; + const originalToLine = f.toLine ?? measureLineCount; + const originalContinuesOnNext = (f as { continuesOnNext?: boolean }).continuesOnNext ?? false; + const secondHalf = { + ...f, + fromLine: splitLine, + toLine: originalToLine, + x: columnX(col + 1), + y: colCursors[col + 1], + width: columnWidth, + continuesFromPrev: true, + continuesOnNext: originalContinuesOnNext, + } as BalancingFragment; + f.toLine = splitLine; + (f as { continuesOnNext?: boolean }).continuesOnNext = true; + colCursors[col] += bp.heightBeforeBreak; + colCursors[col + 1] += bp.heightAfterBreak; + // Insert right after the first half so document order is preserved for + // any later consumer that walks the page fragments. + const fragIdx = fragments.indexOf(f); + if (fragIdx >= 0) fragments.splice(fragIdx + 1, 0, secondHalf); + if (colCursors[col] > maxY) maxY = colCursors[col]; + if (colCursors[col + 1] > maxY) maxY = colCursors[col + 1]; + continue; + } colCursors[col] += block.measuredHeight; if (colCursors[col] > maxY) maxY = colCursors[col]; } diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index 32c4be6e8b..fae4391c71 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -2903,7 +2903,7 @@ describe('layoutDocument', () => { const TWO_COL_RIGHT_X = LEFT_MARGIN + TWO_COL_WIDTH + COLUMN_GAP; // 330 /** Build a 2-col section ending with a section break, surrounded by single-column context. */ - function buildTwoColumnSection(paragraphCount: number, lineHeight = 20) { + function buildTwoColumnSection(paragraphCount: number, lineHeight = 20, endBreakType = 'continuous') { const blocks: FlowBlock[] = [ { kind: 'sectionBreak', @@ -2924,7 +2924,7 @@ describe('layoutDocument', () => { blocks.push({ kind: 'sectionBreak', id: 'sb-end', - type: 'continuous', + type: endBreakType, columns: { count: 1, gap: 0 }, margins: {}, attrs: { source: 'sectPr', sectionIndex: 1 }, @@ -2977,6 +2977,88 @@ describe('layoutDocument', () => { expect(afterSectionPara!.y).toBe(expectedBalancedBottom); }); + it('splits a paragraph straddling the column boundary so the section balances (SD-3359)', () => { + // IT-1150 / SD-3359 repro shape: two 1-line paragraphs plus one 14-line + // paragraph (280px) in a 2-col continuous section followed by continuous + // single-column content. Atomic assignment can only reach 40 | 280; + // Word flows line-by-line, splitting the long paragraph at the boundary + // so both columns reach the minimum section height (§17.18.77): + // col0 = 20 + 20 + 6 lines (160), col1 = 8 lines (160). + const blocks: FlowBlock[] = [ + { + kind: 'sectionBreak', + id: 'sb-start', + type: 'continuous', + columns: { count: 2, gap: COLUMN_GAP }, + margins: {}, + attrs: { source: 'sectPr', sectionIndex: 0, isFirstSection: true }, + } as FlowBlock, + { kind: 'paragraph', id: 'p0', runs: [], attrs: { sectionIndex: 0 } } as FlowBlock, + { kind: 'paragraph', id: 'p1', runs: [], attrs: { sectionIndex: 0 } } as FlowBlock, + { kind: 'paragraph', id: 'p-long', runs: [], attrs: { sectionIndex: 0 } } as FlowBlock, + { + kind: 'sectionBreak', + id: 'sb-end', + type: 'continuous', + columns: { count: 1, gap: 0 }, + margins: {}, + attrs: { source: 'sectPr', sectionIndex: 1 }, + } as FlowBlock, + { kind: 'paragraph', id: 'p-after', runs: [], attrs: { sectionIndex: 1 } } as FlowBlock, + ]; + const measures: Measure[] = [ + { kind: 'sectionBreak' }, + makeMeasure([20]), + makeMeasure([20]), + makeMeasure(Array(14).fill(20)), + { kind: 'sectionBreak' }, + makeMeasure([20]), + ]; + + const layout = layoutDocument(blocks, measures, PAGE); + + const longFrags = layout.pages[0].fragments + .filter((f): f is ParaFragment => f.kind === 'para' && f.blockId === 'p-long') + .sort((a, b) => a.fromLine - b.fromLine); + // The straddling paragraph split into two fragments partitioning its lines. + expect(longFrags).toHaveLength(2); + expect(longFrags[0].toLine).toBe(longFrags[1].fromLine); + expect(longFrags[1].toLine).toBe(14); + expect(longFrags[0].x).toBe(LEFT_MARGIN); + expect(longFrags[1].x).toBe(TWO_COL_RIGHT_X); + // Both columns reach the same minimum height (320 / 2 = 160). + const pAfter = layout.pages[0].fragments.find( + (f): f is ParaFragment => f.kind === 'para' && f.blockId === 'p-after', + ); + expect(pAfter).toBeDefined(); + expect(pAfter!.y).toBe(72 + 160); + }); + + it('does NOT balance a 2-col section whose END break is nextPage (§17.18.77, SD-3359)', () => { + // Per the §17.18.77 note only a CONTINUOUS break balances the previous + // section. A 2-col section that merely STARTS continuous but is ended by + // a nextPage break fills column-by-column instead (Word behavior), and + // the following section starts on a fresh page. Regression for the + // post-layout gate that previously keyed off the section's own begin + // type instead of the break that ends it. + const { blocks, measures } = buildTwoColumnSection(6, 20, 'nextPage'); + + const layout = layoutDocument(blocks, measures, PAGE); + + const sectionFragments = layout.pages[0].fragments.filter( + (f): f is ParaFragment => f.kind === 'para' && f.blockId.startsWith('p') && f.blockId !== 'p-after', + ); + // Unbalanced column-by-column fill: all 6 short paragraphs stay in col 0. + expect(sectionFragments.filter((f) => f.x === LEFT_MARGIN)).toHaveLength(6); + expect(sectionFragments.filter((f) => f.x === TWO_COL_RIGHT_X)).toHaveLength(0); + // The nextPage break pushes the following section to page 2. + expect(layout.pages.length).toBe(2); + const pAfter = layout.pages[1].fragments.find( + (f): f is ParaFragment => f.kind === 'para' && f.blockId === 'p-after', + ); + expect(pAfter).toBeDefined(); + }); + it('fills BOTH columns on every page of a multi-page 2-col continuous section', () => { // ECMA-376 §17.18.77 (ST_SectionMark): a continuous section break // "balances content of the previous section." Word's observable behavior diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 89f356b7b3..a9147b5742 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -1851,13 +1851,13 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options const blockSectionMap = new Map(); const sectionColumnsMap = new Map(); const sectionHasExplicitColumnBreak = new Set(); - // sectionIndex -> type of the section break that ENDS this section (per - // pm-adapter end-tagged semantics, ECMA-376 §17.6.17: a paragraph's sectPr - // describes the section ENDING at that paragraph, so SectionBreakBlock.type - // here is the type of the break that closes the section). Per ECMA-376 - // §17.18.77 only `continuous` breaks trigger column balancing — `nextPage`, - // `evenPage`, `oddPage` do not. Tracked here so the post-layout pass can - // skip the wrong section types. + // sectionIndex -> the section's own sectPr `w:type` (ECMA-376 §17.6.22): + // how the section BEGINS relative to its predecessor — i.e. the type of the + // break that closes the PREVIOUS section. The break that ENDS section N is + // therefore `get(N + 1)`. Per ECMA-376 §17.18.77 only a `continuous` break + // balances the section BEFORE it — `nextPage`, `evenPage`, `oddPage` do + // not. (The earlier comment here claimed end-break semantics, which led the + // post-layout gate to key off the wrong section — SD-3359.) const sectionEndBreakType = new Map(); // sectionIndex -> whether `` was EXPLICIT in the source sectPr. // Body sectPrs default to `continuous` when w:type is omitted; Word does @@ -1882,6 +1882,10 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // the older `line.width === 0` heuristic, which incorrectly collapsed normal // blank paragraphs and caused overlap on the next paragraph. const sectPrMarkerBlockIds = new Set(); + // Block IDs of paragraphs with `w:keepLines` (ECMA-376 §17.3.1.14): the + // author asked Word not to split these, so column balancing must keep them + // atomic instead of breaking them at a line boundary. (SD-3359) + const keepLinesBlockIds = new Set(); // True if any block in the document is a column break. Used as a guard for // the document-wide balancing fallback (Nick comment 2): when callers use // LayoutOptions.columns without section metadata, we still want Word's @@ -1946,6 +1950,12 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options ) { sectPrMarkerBlockIds.add(block.id); } + if ( + block.kind === 'paragraph' && + (blockWithAttrs as { attrs?: { keepLines?: boolean } }).attrs?.keepLines === true + ) { + keepLinesBlockIds.add(block.id); + } }); // Collect anchored drawings mapped to their anchor paragraphs @@ -2347,6 +2357,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options availableHeight, measureMap: balancingMeasureMap, sectPrMarkerBlockIds, + keepLinesBlockIds, }); if (balanceResult) { // Collapse both cursors to the balanced section bottom so the new @@ -3105,7 +3116,19 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // (two_column_two_page-arial 2 p17 keeps its 3+2 split). if (isMultiPage && !isLast) continue; - const allowedByMidDocContinuous = endBreakType === 'continuous' && !isLast; + // The per-section type is the type of the break that BEGINS the section + // (its own sectPr `w:type`, §17.6.22) — i.e. the break that closes the + // PREVIOUS section. The break that ends section N is therefore section + // N+1's begin type. Keying rule 1 off the section's OWN type balanced a + // 2-col section that merely STARTED continuous even when it ended at a + // nextPage break — Word only balances when the break AFTER the section + // is continuous (§17.18.77 note, SD-3359 V6 repro). The next-is-body + // case is excluded here: a body sectPr defaults to `continuous` when + // `` is omitted and Word does NOT balance then (sd-1655) — + // rule 2 below owns that boundary and demands explicitness. + const nextSectionBeginType = sectionEndBreakType.get(sectionIdx + 1); + const nextIsBody = lastSectionIdx !== null && sectionIdx + 1 === lastSectionIdx; + const allowedByMidDocContinuous = !isLast && !nextIsBody && nextSectionBeginType === 'continuous'; // Body-explicit-continuous balances the section IT ENDS, which is the // section immediately preceding the body. No doc-wide flag. const allowedByBodyExplicitContinuous = @@ -3159,6 +3182,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options availableHeight: sectionAvailableHeight, measureMap: balancingMeasureMap, sectPrMarkerBlockIds, + keepLinesBlockIds, }); } From f19296475b287563908893a767b76abeccb39c57 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Fri, 5 Jun 2026 10:37:47 -0300 Subject: [PATCH 6/6] fix(layout-engine): honor remeasured fragment lines when splitting columns Review follow-up. A paragraph fragment remeasured for a narrower column (or beside a float) carries its own `lines` array, and resolveParagraph renders that array INSTEAD of slicing measure.lines by fromLine/toLine. The column split cloned the fragment wholesale, so both halves kept the full remeasured array and each column rendered the entire paragraph. Three coordinated corrections: - The split slices `lines` across the halves (first keeps the leading slice, the clone carries the rest), so each column renders only its own lines. - Break points are computed against the fragment's own remeasured line heights when present; measure.lines describes the original width and can disagree in both count and height. - getFragmentHeight sums fragment.lines when present, matching how resolveLayout sizes such fragments, so balancing cursors agree with what the resolve stage actually renders. Adds a regression test with a remeasured fragment (22px lines vs a stale 20px measure) asserting the halves partition the lines and the column cursors advance by the remeasured heights. --- .../src/column-balancing.test.ts | 36 +++++++++++++++++++ .../layout-engine/src/column-balancing.ts | 25 +++++++++++++ 2 files changed, 61 insertions(+) diff --git a/packages/layout-engine/layout-engine/src/column-balancing.test.ts b/packages/layout-engine/layout-engine/src/column-balancing.test.ts index f0d04c3538..9da8094774 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.test.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.test.ts @@ -653,6 +653,42 @@ describe('balanceSectionOnPage', () => { expect(result!.maxY).toBeLessThan(TOP + 280); }); + it('slices remeasured fragment.lines across the split (no duplicated halves)', () => { + // A fragment remeasured for a narrower column carries its own `lines`, and + // resolveParagraph renders that array INSTEAD of measure.lines[fromLine..toLine]. + // The split must slice `lines` for each half, or both columns render the whole + // paragraph. The remeasured heights (22px) also differ from the stale measure + // (20px), so the break point and cursors must come from the remeasured lines. + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + const REMEASURED = 22; + const c = fragments[2] as SplitFragment & { lines?: Array<{ lineHeight: number }> }; + c.lines = Array.from({ length: 14 }, () => ({ lineHeight: REMEASURED })); + + const result = balance(fragments, measureMap, blockSectionMap); + + expect(result).not.toBeNull(); + const cFrags = ( + fragments.filter((f) => f.blockId === 'C') as Array }> + ).sort((a, b) => (a.fromLine ?? 0) - (b.fromLine ?? 0)); + expect(cFrags.length).toBe(2); + const [c1, c2] = cFrags; + // Each half carries ONLY its own remeasured lines, partitioning the original 14. + expect(c1.lines).toBeDefined(); + expect(c2.lines).toBeDefined(); + expect(c1.lines!.length + c2.lines!.length).toBe(14); + expect(c1.lines!.length).toBe((c1.toLine ?? 0) - (c1.fromLine ?? 0)); + expect(c2.lines!.length).toBe(c2.toLine! - c2.fromLine!); + // Cursors advanced by the remeasured heights: the second column's bottom is + // its line count at 22px, not at the stale 20px measure. + const col1Frags = fragments.filter((f) => f.x === COL1_X) as Array< + SplitFragment & { lines?: Array<{ lineHeight: number }> } + >; + const col1Bottom = Math.max( + ...col1Frags.map((f) => f.y + (f.lines ? f.lines.reduce((s, l) => s + l.lineHeight, 0) : 0)), + ); + expect(col1Bottom).toBe(result!.maxY); + }); + it('offsets the split by the fragment fromLine when pagination already split the paragraph', () => { const { fragments, measureMap, blockSectionMap } = straddleFixture(); // C is the tail of a 16-line paragraph: this page renders lines [2, 16). diff --git a/packages/layout-engine/layout-engine/src/column-balancing.ts b/packages/layout-engine/layout-engine/src/column-balancing.ts index e3c3f779d0..8210bc194b 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.ts @@ -513,6 +513,13 @@ export interface BalancingFragment { fromLine?: number; /** Ending line index (exclusive) for partial paragraph fragments */ toLine?: number; + /** + * Remeasured lines carried by the fragment itself (set when a paragraph measured at one + * width is placed in a narrower column or beside a float). When present, the resolve + * stage renders THIS array and ignores fromLine/toLine into measure.lines - so balancing + * must source heights from it and slice it when splitting a fragment across columns. + */ + lines?: Array<{ lineHeight: number }>; /** Pre-computed height for non-paragraph fragments */ height?: number; } @@ -556,6 +563,11 @@ interface FragmentInfo { */ function getFragmentHeight(fragment: BalancingFragment, measureMap: Map): number { if (fragment.kind === 'para') { + // A fragment remeasured for a narrower column carries its own lines; the resolve + // stage renders (and sizes) from THAT array, so balancing must agree with it. + if (fragment.lines && fragment.lines.length > 0) { + return fragment.lines.reduce((sum, l) => sum + (l.lineHeight ?? 0), 0); + } const measure = measureMap.get(fragment.blockId); if (!measure || measure.kind !== 'paragraph' || !measure.lines) { return 0; @@ -803,6 +815,12 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu if (f.kind !== 'para') return undefined; if (args.sectPrMarkerBlockIds?.has(f.blockId)) return undefined; if (args.keepLinesBlockIds?.has(f.blockId)) return undefined; + // A remeasured fragment renders its own `lines` (resolveParagraph ignores + // fromLine/toLine then), so break points must be computed against that array. + if (f.lines && f.lines.length > 0) { + if (f.lines.length <= 1) return undefined; + return f.lines.map((l) => l.lineHeight); + } const measure = args.measureMap.get(f.blockId); if (!measure || measure.kind !== 'paragraph' || !Array.isArray(measure.lines)) return undefined; const fromLine = f.fromLine ?? 0; @@ -876,6 +894,13 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu continuesFromPrev: true, continuesOnNext: originalContinuesOnNext, } as BalancingFragment; + // Remeasured fragments render their own `lines` wholesale (fromLine/toLine are + // ignored by the resolve stage then), so the halves must each carry ONLY their + // slice or both columns render the entire paragraph. + if (f.lines && f.lines.length > 0) { + secondHalf.lines = f.lines.slice(bp.breakAfterLine + 1); + f.lines = f.lines.slice(0, bp.breakAfterLine + 1); + } f.toLine = splitLine; (f as { continuesOnNext?: boolean }).continuesOnNext = true; colCursors[col] += bp.heightBeforeBreak;