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/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; } 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..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 @@ -288,6 +288,338 @@ 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('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('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', 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..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,20 +255,43 @@ 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']; + // 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 = 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 } : {}), };