diff --git a/packages/layout-engine/contracts/src/column-layout.test.ts b/packages/layout-engine/contracts/src/column-layout.test.ts index 5aca69b3e8..0c485c483d 100644 --- a/packages/layout-engine/contracts/src/column-layout.test.ts +++ b/packages/layout-engine/contracts/src/column-layout.test.ts @@ -15,6 +15,7 @@ import { resolveColumnLayout, resolveColumnMode, widthsEqual, + findColumnContaining, } from './column-layout.js'; describe('widthsEqual', () => { @@ -88,6 +89,7 @@ describe('normalizeColumnLayout', () => { gap: 0, widths: [480], width: 480, + contentWidth: 480, }); }); @@ -97,6 +99,7 @@ describe('normalizeColumnLayout', () => { gap: 24, widths: [300, 300], width: 300, + contentWidth: 624, }); }); @@ -109,6 +112,7 @@ describe('normalizeColumnLayout', () => { widths: [100, 200], equalWidth: false, width: 200, + contentWidth: 624, }); }); @@ -123,6 +127,7 @@ describe('normalizeColumnLayout', () => { widths: [200, 400], equalWidth: false, width: 400, + contentWidth: 300, }); }); @@ -133,6 +138,7 @@ describe('normalizeColumnLayout', () => { gap: 24, widths: [300, 300], width: 300, + contentWidth: 624, }); }); @@ -143,6 +149,7 @@ describe('normalizeColumnLayout', () => { widths: [300, 300], equalWidth: true, width: 300, + contentWidth: 624, }); }); @@ -155,6 +162,7 @@ describe('normalizeColumnLayout', () => { widths: [192, 384], equalWidth: false, width: 384, + contentWidth: 624, }); }); @@ -163,6 +171,7 @@ describe('normalizeColumnLayout', () => { count: 1, gap: 0, width: 0, + contentWidth: 0, }); }); }); @@ -373,15 +382,143 @@ describe('columnRenderLayoutsEqual (SD-2629)', () => { expect(columnRenderLayoutsEqual({ count: 2, gap: 24 }, { count: 2, gap: 48 })).toBe(false); }); - it('treats explicit layouts differing only by per-column gaps as render-equal until geometry flips', () => { + it('splits explicit layouts that differ only by per-column gaps', () => { + // Geometry has flipped: `buildColumnGeometry` reads `gaps[i] ?? gap` for both the column x and + // the separator x, so this delta moves column 2 by 72px. While it compared equal, two sections + // differing only here shared a region and the later one was laid out with the earlier's gutters. expect( columnRenderLayoutsEqual( { count: 3, gap: 24, widths: [100, 100, 300], gaps: [24, 24], equalWidth: false }, { count: 3, gap: 24, widths: [100, 100, 300], gaps: [24, 96], equalWidth: false }, ), + ).toBe(false); + + // Identical per-column gaps still compare equal, and so does an absent-vs-absent pair. + expect( + columnRenderLayoutsEqual( + { count: 3, gap: 24, widths: [100, 100, 300], gaps: [24, 96], equalWidth: false }, + { count: 3, gap: 24, widths: [100, 100, 300], gaps: [24, 96], equalWidth: false }, + ), + ).toBe(true); + expect( + columnRenderLayoutsEqual( + { count: 3, gap: 24, widths: [100, 100, 300], equalWidth: false }, + { count: 3, gap: 24, widths: [100, 100, 300], equalWidth: false }, + ), + ).toBe(true); + + // Equal mode has no per-column gaps to compare, so stray ones must not split a region. + expect(columnRenderLayoutsEqual({ count: 2, gap: 24, gaps: [24] }, { count: 2, gap: 24, gaps: [96] })).toBe(true); + }); + + it('compares the gutters that are drawn, not the authored arrays', () => { + const explicit = (gaps?: number[]): ColumnLayout => ({ + count: 3, + gap: 48, + widths: [100, 100, 300], + equalWidth: false, + ...(gaps ? { gaps } : {}), + }); + + // Spelling the scalar gap out per column renders identically, so it must NOT split a region. + // Comparing the authored arrays instead would see `undefined` vs `[48, 48]` and split, resetting + // the later section to column 0 mid-page. + expect(columnRenderLayoutsEqual(explicit(), explicit([48, 48]))).toBe(true); + // Same for a negative gutter, which geometry floors at 0 exactly as 0 does. + expect(columnRenderLayoutsEqual(explicit([48, 0]), explicit([48, -10]))).toBe(true); + + // A SHORT array falls back to the scalar gap for the gutter it omits — it does not mean 0. These + // two render differently and must split; padding the authored array with 0 would call them equal. + expect(columnRenderLayoutsEqual(explicit([20]), explicit([20, 0]))).toBe(false); + expect(columnRenderLayoutsEqual(explicit([20]), explicit([20, 48]))).toBe(true); + + // Surplus gaps beyond count-1 are discarded by resolution, so they must not split either. + expect(columnRenderLayoutsEqual(explicit([48, 48, 999]), explicit([48, 48]))).toBe(true); + }); + + it('ignores a scalar gap that explicit per-column gaps have made unreachable', () => { + const explicit = (gap: number, gaps?: number[]): ColumnLayout => ({ + count: 3, + gap, + widths: [100, 100, 300], + equalWidth: false, + ...(gaps ? { gaps } : {}), + }); + + // `gaps` supplies BOTH gutters, so geometry never reaches the scalar fallback and these two draw + // the same three columns in the same places. The scalar comparison used to run ahead of the mode + // branch and split anyway — a mid-page region break plus a cache flush over a value nothing + // read, which restarts the following content in column 0. + expect(columnRenderLayoutsEqual(explicit(24, [30, 40]), explicit(900, [30, 40]))).toBe(true); + + // The moment the array stops supplying a gutter, the scalar IS that gutter and has to + // discriminate again. + expect(columnRenderLayoutsEqual(explicit(24, [30]), explicit(900, [30]))).toBe(false); + expect(columnRenderLayoutsEqual(explicit(24), explicit(900))).toBe(false); + + // Equal mode reads the scalar twice over — as every gutter, and as what normalization subtracts + // before dividing the content area — so it still splits there. + expect(columnRenderLayoutsEqual({ count: 3, gap: 24 }, { count: 3, gap: 40 })).toBe(false); + }); + + it('falls a hole in the gaps array back to the scalar, not to NaN', () => { + // `Math.max(0, undefined)` is NaN, and NaN is not nullish, so `buildColumnGeometry`'s + // `gaps?.[i] ?? gap` could not rescue it: the hole propagated into every later column's x. + // A hole is forbidden by `gaps?: number[]` and nothing in the repo builds a gaps array yet, so + // this is a guard for the day the importer starts projecting `w:space` per column. + const holed = normalizeColumnLayout( + { count: 3, gap: 40, widths: [200, 200, 200], equalWidth: false, gaps: [30, undefined] } as ColumnLayout, + 720, + ); + expect(holed.gaps).toEqual([30, 40]); + const geometry = getColumnGeometry(holed); + expect(geometry.map((col) => col.x)).toEqual([0, 230, 470]); + expect(geometry.every((col) => Number.isFinite(col.x))).toBe(true); + + // Which also means the hole now renders identically to spelling the scalar out, so the render + // predicate must call the two equal rather than splitting a region over an unreachable value. + expect( + columnRenderLayoutsEqual( + { count: 3, gap: 40, widths: [200, 200, 200], equalWidth: false, gaps: [30, undefined] } as ColumnLayout, + { count: 3, gap: 40, widths: [200, 200, 200], equalWidth: false, gaps: [30, 40] }, + ), ).toBe(true); }); + it('still splits explicit sub-pixel widths on the scalar gap alone', () => { + // The one route by which the scalar reaches explicit WIDTHS rather than the gutters: normalize + // floors a fabricated width at 1px, and collapses to a single full-width column once the usable + // width falls to epsilon — both keyed on the sign of `contentWidth - gap * (count - 1)`, which + // the scalar moves. It can only bite when an authored width is ITSELF sub-pixel, because at 1px + // or more the floor and the collapse are no-ops, which is why the two tests above are unaffected. + // + // Measured at contentWidth 720: `[0.5, 0.5, 0.5]` renders as three 1px columns under gap 24 and + // as three 0.5px ones under gap 999, and at 1e-5 the second collapses to one 720px column. So + // these pairs must NOT compare equal, even though `gaps` supplies every gutter. + const subPixel = (gap: number): ColumnLayout => ({ + count: 3, + gap, + widths: [0.5, 0.5, 0.5], + equalWidth: false, + gaps: [1, 1], + }); + expect(columnRenderLayoutsEqual(subPixel(24), subPixel(999))).toBe(false); + // Same scalar on both sides is still equal — the guard keys on the gap differing, not on the + // widths being small. + expect(columnRenderLayoutsEqual(subPixel(24), subPixel(24))).toBe(true); + + // A whole-pixel width is immune, which is the property that keeps this off every real document: + // no `w:col/@w` a document can author lands under 1px except at absurd twip values. + const wholePixel = (gap: number): ColumnLayout => ({ + count: 3, + gap, + widths: [1, 1, 1], + equalWidth: false, + gaps: [1, 1], + }); + expect(columnRenderLayoutsEqual(wholePixel(24), wholePixel(999))).toBe(true); + }); + it('distinguishes explicit vs equal mode and different resolved widths', () => { expect( columnRenderLayoutsEqual({ count: 2, gap: 24, widths: [192, 384], equalWidth: false }, { count: 2, gap: 24 }), @@ -399,3 +536,287 @@ describe('columnRenderLayoutsEqual (SD-2629)', () => { expect(columnRenderLayoutsEqual({ count: 2, gap: 24 }, undefined)).toBe(false); }); }); + +describe('RTL section column order', () => { + /** A4 body: 602px of content, two equal columns, 48px gutter (720tw). */ + const twoEqual = (direction?: 'ltr' | 'rtl'): ColumnLayout => ({ + count: 2, + gap: 48, + ...(direction ? { direction } : {}), + }); + + it('puts the first column on the right without reordering indices', () => { + const ltr = getColumnGeometry(normalizeColumnLayout(twoEqual(), 602)); + const rtl = getColumnGeometry(normalizeColumnLayout(twoEqual('rtl'), 602)); + + // Fill order is the index; only the painted x moves. + expect(ltr.map((col) => col.index)).toEqual([0, 1]); + expect(rtl.map((col) => col.index)).toEqual([0, 1]); + expect(ltr.map((col) => col.x)).toEqual([0, 325]); + expect(rtl.map((col) => col.x)).toEqual([325, 0]); + // Widths and the strip's total span are untouched by the mirror. + expect(rtl.map((col) => col.width)).toEqual(ltr.map((col) => col.width)); + expect(Math.max(...rtl.map((col) => col.x + col.width))).toBe(602); + }); + + it('leaves an LTR layout exactly where it was', () => { + // The regression that matters most: every existing producer omits `direction`. + expect(getColumnGeometry(normalizeColumnLayout(twoEqual(), 602))).toEqual( + getColumnGeometry(normalizeColumnLayout({ count: 2, gap: 48 }, 602)), + ); + }); + + it('is a no-op for a single column that fills the content area', () => { + const rtl = getColumnGeometry(normalizeColumnLayout({ count: 1, gap: 48, direction: 'rtl' }, 602)); + expect(rtl).toEqual([{ index: 0, x: 0, width: 602, gapAfter: 0 }]); + }); + + it('pins a single underfilling explicit column to the RIGHT margin', () => { + // One column has no order to flip, but it still has a side. `` + // with an authored width narrower than the body leaves slack, and in an RTL section that slack + // belongs on the left — the same axis rule the multi-column strip follows. + const rtl = getColumnGeometry( + normalizeColumnLayout({ count: 1, gap: 0, equalWidth: false, widths: [200], direction: 'rtl' }, 602), + ); + expect(rtl).toEqual([{ index: 0, x: 402, width: 200, gapAfter: 0 }]); + + // LTR keeps the slack on the right, as before. + const ltr = getColumnGeometry(normalizeColumnLayout({ count: 1, gap: 0, equalWidth: false, widths: [200] }, 602)); + expect(ltr).toEqual([{ index: 0, x: 0, width: 200, gapAfter: 0 }]); + }); + + it('mirrors three columns with per-column gaps onto the right physical gutters', () => { + const columns: ColumnLayout = { + count: 3, + gap: 0, + equalWidth: false, + widths: [100, 150, 200], + gaps: [20, 40], + withSeparator: true, + direction: 'rtl', + }; + const rtl = getColumnGeometry(normalizeColumnLayout(columns, 602)); + + // Fill order still runs 0,1,2; the strip is laid out right to left from the right margin. + expect(rtl.map((col) => col.index)).toEqual([0, 1, 2]); + expect(rtl.map((col) => col.x)).toEqual([502, 332, 92]); + // Column 0's right edge is the right margin, and each separator is the midpoint of the gutter + // between the columns it actually separates. + expect(rtl[0].x + rtl[0].width).toBe(602); + expect(getColumnSeparatorPositions(rtl, 0)).toEqual([492, 312]); + // Hit testing descends with the index and every column claims its own span. + expect(getColumnAtX(rtl, 550)).toBe(0); + expect(getColumnAtX(rtl, 400)).toBe(1); + expect(getColumnAtX(rtl, 150)).toBe(2); + }); + + it('clamps a negative per-column gap so an LTR layout cannot read as mirrored', () => { + // OOXML cannot express a negative gutter (`w:space` is unsigned), but a host-built layout can. + // Left unclamped, `gaps: [-100]` pulls column 1 back behind column 0 and the direction-aware + // consumers — which infer the axis from x monotonicity — would answer hit tests as if the + // upright layout were mirrored. + const ltr = getColumnGeometry( + normalizeColumnLayout({ count: 2, gap: 0, equalWidth: false, widths: [50, 50], gaps: [-100] }, 602), + ); + expect(ltr.map((col) => col.x)).toEqual([0, 50]); + expect(getColumnAtX(ltr, 20)).toBe(0); + expect(getColumnAtX(ltr, 80)).toBe(1); + }); + + it('pins an underfilling explicit strip to the RIGHT margin, not the left', () => { + // Word does not scale authored widths to fill the content area, so two 192px columns in a 602px + // body leave 170px of slack. In LTR the slack falls on the right; mirrored, it must fall on the + // left. Mirroring about the strip's own span instead of the content area would leave the whole + // strip pinned left and merely swap the columns inside it — the document would still read as + // left-aligned, which is the bug this whole change exists to fix. + const columns: ColumnLayout = { + count: 2, + gap: 48, + equalWidth: false, + widths: [192, 192], + direction: 'rtl', + }; + const rtl = getColumnGeometry(normalizeColumnLayout(columns, 602)); + + expect(rtl.map((col) => col.x)).toEqual([410, 170]); + // Column 0's right edge is the right margin; the slack is on the left. + expect(rtl[0].x + rtl[0].width).toBe(602); + expect(Math.min(...rtl.map((col) => col.x))).toBe(170); + }); + + it('lets an overfull explicit strip run past the LEFT margin', () => { + // The mirror image of the documented LTR overflow: authored widths are not scaled down either, + // so the strip runs off the far margin — which in RTL is the left one. + const columns: ColumnLayout = { + count: 2, + gap: 24, + equalWidth: false, + widths: [200, 400], + direction: 'rtl', + }; + const rtl = getColumnGeometry(normalizeColumnLayout(columns, 300)); + + expect(rtl[0].x + rtl[0].width).toBe(300); + expect(rtl[1].x).toBe(-324); + }); + + it('mirrors the separator onto the same physical gutter', () => { + const columns: ColumnLayout = { + count: 2, + gap: 50, + equalWidth: false, + widths: [200, 352], + withSeparator: true, + direction: 'rtl', + }; + const rtl = getColumnGeometry(normalizeColumnLayout(columns, 602)); + + // Column 1 (left) spans [0,352], column 0 (right) spans [402,602]; the gutter is 352..402 + // and the separator sits at its midpoint. + expect(rtl[0]).toEqual({ index: 0, x: 402, width: 200, gapAfter: 50, separatorX: 377 }); + expect(rtl[1]).toEqual({ index: 1, x: 0, width: 352, gapAfter: 0 }); + expect(getColumnSeparatorPositions(rtl, 96)).toEqual([473]); + }); + + it('resolves a point to the column that visually contains it', () => { + const rtl = getColumnGeometry(normalizeColumnLayout(twoEqual('rtl'), 602)); + + // Right half is the FIRST column now; left half is the second. + expect(getColumnAtX(rtl, 400)).toBe(0); + expect(getColumnAtX(rtl, 100)).toBe(1); + // Edges stay inside their own column. + expect(getColumnAtX(rtl, 602)).toBe(0); + expect(getColumnAtX(rtl, 0)).toBe(1); + // A point in the gutter belongs to the column preceding it in fill order — the same rule the + // LTR branch applies, mirrored. This is what keeps a drag crossing the gutter from jumping. + expect(getColumnAtX(rtl, 300)).toBe(0); + }); + + it('resolves an RTL column boundary the same way containment does', () => { + // `w:space="0"` (ECMA-376 §17.6.3) makes adjacent columns share an edge, which is the one point + // hit testing and containment can be made to disagree about. Two columns over 602px mirror to + // column 0 at [301,602) and column 1 at [0,301), so 301 is column 0's leading edge and column + // 1's trailing one at the same time. + const flush = getColumnGeometry(normalizeColumnLayout({ count: 2, gap: 0, direction: 'rtl' }, 602)); + expect(flush.map((col) => col.x)).toEqual([301, 0]); + + // Half-open spans put a shared edge in the column that STARTS there, which in RTL is the earlier + // column in fill order. An inclusive mirrored bound handed it to column 1 instead, so every + // column boundary in a zero-gutter RTL section resolved one column too far — and disagreed with + // the containment the geometry places content by. + expect(findColumnContaining(flush, 301)).toBe(0); + expect(getColumnAtX(flush, 301)).toBe(0); + // A hair to the left is genuinely column 1's, in both resolvers. + expect(findColumnContaining(flush, 300.99)).toBe(1); + expect(getColumnAtX(flush, 300.99)).toBe(1); + + // With a gutter the same bound over-claimed the point on a column's TRAILING edge, which is + // gutter and belongs to the column preceding it in fill order. + const gutter = getColumnGeometry(normalizeColumnLayout(twoEqual('rtl'), 602)); + expect(gutter[1]).toEqual({ index: 1, x: 0, width: 277, gapAfter: 0 }); + expect(findColumnContaining(gutter, 277)).toBeNull(); + expect(getColumnAtX(gutter, 277)).toBe(0); + expect(getColumnAtX(gutter, 276.99)).toBe(1); + }); + + it('keeps LTR hit testing byte-identical', () => { + const ltr = getColumnGeometry(normalizeColumnLayout(twoEqual(), 602)); + expect(getColumnAtX(ltr, 100)).toBe(0); + expect(getColumnAtX(ltr, 300)).toBe(0); + expect(getColumnAtX(ltr, 400)).toBe(1); + }); + + it('honors originX in both directions', () => { + const rtl = getColumnGeometry(normalizeColumnLayout(twoEqual('rtl'), 602)); + expect(getColumnX(rtl, 0, 96)).toBe(421); + expect(getColumnX(rtl, 1, 96)).toBe(96); + expect(getColumnAtX(rtl, 500, 96)).toBe(0); + expect(getColumnAtX(rtl, 200, 96)).toBe(1); + }); + + it('carries direction through clone and normalize', () => { + expect(cloneColumnLayout(twoEqual('rtl')).direction).toBe('rtl'); + expect(cloneColumnLayout(twoEqual()).direction).toBeUndefined(); + expect(normalizeColumnLayout(twoEqual('rtl'), 602).direction).toBe('rtl'); + expect(resolveColumnLayout(twoEqual('rtl')).direction).toBe('rtl'); + }); + + it('treats direction as paint-significant in both equality checks', () => { + // A section that only flips direction must split regions and invalidate the cache; treating it + // as equal would leave the previous geometry painted. + expect(columnLayoutsEqual(twoEqual('rtl'), twoEqual('ltr'))).toBe(false); + expect(columnRenderLayoutsEqual(twoEqual('rtl'), twoEqual('ltr'))).toBe(false); + // Absent means ltr, so omitting it must not read as a change. + expect(columnLayoutsEqual(twoEqual(), twoEqual('ltr'))).toBe(true); + expect(columnRenderLayoutsEqual(twoEqual(), twoEqual('ltr'))).toBe(true); + }); +}); + +describe('findColumnContaining', () => { + // 3 equal columns over 624px with a 24px gap: 192px columns at 0, 216, 432. + const ltr = getColumnGeometry(normalizeColumnLayout({ count: 3, gap: 24 }, 624)); + const rtl = getColumnGeometry(normalizeColumnLayout({ count: 3, gap: 24, direction: 'rtl' }, 624)); + + it('resolves an x inside a column to that column, in both directions', () => { + expect(findColumnContaining(ltr, 10)).toBe(0); + expect(findColumnContaining(ltr, 300)).toBe(1); + expect(findColumnContaining(ltr, 500)).toBe(2); + // Mirrored: column 0 is the rightmost, so the same points answer in reverse. + expect(findColumnContaining(rtl, 10)).toBe(2); + expect(findColumnContaining(rtl, 300)).toBe(1); + expect(findColumnContaining(rtl, 500)).toBe(0); + }); + + it('answers null in a gutter instead of clamping to a neighbour', () => { + // The gap between column 0 and 1 runs 192..216 in LTR. + expect(findColumnContaining(ltr, 200)).toBeNull(); + // getColumnAtX, which exists for hit testing, must still clamp there. + expect(getColumnAtX(ltr, 200)).toBe(0); + }); + + it('answers null outside the strip entirely, in both directions', () => { + expect(findColumnContaining(ltr, -50)).toBeNull(); + expect(findColumnContaining(ltr, 700)).toBeNull(); + expect(findColumnContaining(rtl, -50)).toBeNull(); + expect(findColumnContaining(rtl, 700)).toBeNull(); + }); + + it('identifies a fragment WIDER than its column by its origin', () => { + // An over-wide table is placed at its column's left edge and overflows rightward in BOTH + // directions. Its origin still names its column; its trailing edge does not, which is exactly + // why an edge comparison cannot answer this question. + const originOfLastColumn = rtl[2].x; + expect(findColumnContaining(rtl, originOfLastColumn)).toBe(2); + // The same fragment's right edge, 500px later, has left the column and reads as another one. + expect(findColumnContaining(rtl, originOfLastColumn + 500)).not.toBe(2); + }); + + it('gives a shared zero-gap boundary to the column that STARTS there', () => { + // `w:space="0"` makes adjacent columns share an endpoint, and that endpoint is exactly where + // the later column's content is placed. Inclusive spans would hand it to the column that ends + // there instead, and — because the scan runs in fill order — would do so in LTR but not in RTL, + // making the two directions disagree. + const zeroGap = getColumnGeometry(normalizeColumnLayout({ count: 2, gap: 0 }, 624)); + expect(zeroGap.map((col) => col.x)).toEqual([0, 312]); + expect(findColumnContaining(zeroGap, 311.9)).toBe(0); + expect(findColumnContaining(zeroGap, 312)).toBe(1); + + // The mirrored strip has to answer the same way about its own shared boundary. + const zeroGapRtl = getColumnGeometry(normalizeColumnLayout({ count: 2, gap: 0, direction: 'rtl' }, 624)); + expect(zeroGapRtl.map((col) => col.x)).toEqual([312, 0]); + expect(findColumnContaining(zeroGapRtl, 312)).toBe(0); + expect(findColumnContaining(zeroGapRtl, 311.9)).toBe(1); + }); + + it('honors originX', () => { + expect(findColumnContaining(ltr, 106, 96)).toBe(0); + expect(findColumnContaining(ltr, 96 + 300, 96)).toBe(1); + expect(findColumnContaining(ltr, 0, 96)).toBeNull(); + }); + + it('treats a single column as a column', () => { + const one = getColumnGeometry(normalizeColumnLayout({ count: 1, gap: 0 }, 624)); + expect(findColumnContaining(one, 300)).toBe(0); + expect(findColumnContaining(one, 900)).toBeNull(); + }); +}); diff --git a/packages/layout-engine/contracts/src/column-layout.ts b/packages/layout-engine/contracts/src/column-layout.ts index 83474a655f..712e448c2f 100644 --- a/packages/layout-engine/contracts/src/column-layout.ts +++ b/packages/layout-engine/contracts/src/column-layout.ts @@ -1,4 +1,4 @@ -import type { ColumnLayout } from './index.js'; +import type { BaseDirection, ColumnLayout } from './index.js'; /** * Resolved geometry for a single column. `x` and `separatorX` are CONTENT-RELATIVE (measured from @@ -15,7 +15,24 @@ export type ColumnGeometry = { separatorX?: number; }; -export type NormalizedColumnLayout = ColumnLayout & { width: number }; +export type NormalizedColumnLayout = ColumnLayout & { + width: number; + /** + * The content-area width the layout was normalized against, in px. + * + * Only RTL geometry reads it, and it exists because `width` above is the WIDEST column, not the + * strip: explicit widths are deliberately not scaled to fill the content area (Word renders an + * authored 2880tw column as 2880tw and leaves the slack), so a strip of explicit columns can be + * narrower — or wider — than the area it sits in. Mirroring such a strip about its own span would + * keep it pinned to the LEFT margin and only swap the columns inside it, which is not what Word + * does: the first column belongs against the RIGHT margin and the slack falls on the left. + * + * Optional because `getColumnGeometry` also accepts hand-built layouts (column balancing assembles + * one directly). When absent, RTL mirrors about the strip's own span, which is exact whenever the + * columns fill the area — always true in equal mode. + */ + contentWidth?: number; +}; export function widthsEqual(a?: number[], b?: number[]): boolean { if (!a && !b) return true; @@ -69,6 +86,7 @@ export function cloneColumnLayout(columns?: ColumnLayout): ColumnLayout { ...(Array.isArray(columns.gaps) ? { gaps: [...columns.gaps] } : {}), ...(columns.equalWidth !== undefined ? { equalWidth: columns.equalWidth } : {}), ...(columns.withSeparator !== undefined ? { withSeparator: columns.withSeparator } : {}), + ...(columns.direction !== undefined ? { direction: columns.direction } : {}), } : { count: 1, gap: 0 }; } @@ -116,7 +134,14 @@ export function resolveColumnLayout(input: ColumnLayout): ColumnLayout { * own `gaps[i]` when provided (SD-2629 step 4), falling back to the uniform scalar gap; the last * column has no following gap. The separator sits at the midpoint of that column's own gap. */ -function buildColumnGeometry(widths: number[], gap: number, withSeparator: boolean, gaps?: number[]): ColumnGeometry[] { +function buildColumnGeometry( + widths: number[], + gap: number, + withSeparator: boolean, + gaps?: number[], + direction?: BaseDirection, + contentWidth?: number, +): ColumnGeometry[] { const geometry: ColumnGeometry[] = []; let x = 0; for (let i = 0; i < widths.length; i += 1) { @@ -128,7 +153,29 @@ function buildColumnGeometry(widths: number[], gap: number, withSeparator: boole geometry.push(col); x += width + gapAfter; } - return geometry; + if (direction !== 'rtl') return geometry; + + // RTL: the FIRST column belongs on the right (ECMA-376 §17.6.1). A single column is mirrored too: + // it is a no-op when the column fills the content area, but an explicit column that underfills it + // still belongs against the RIGHT margin, by the same axis rule as a multi-column strip. + // + // Mirror rather than reverse the array: `index` stays the FILL order, so every consumer that + // walks columns 0..n-1 keeps filling in document order and only the painted x changes. `x` stays + // the LEFT edge of the column, which is what the whole geometry API and its callers mean by `x`. + // `gapAfter` is likewise untouched — it is the gap after this column in fill order, and in RTL + // that gap lies to its left, exactly where the mirrored x places it. + // + // The mirror axis is the CONTENT AREA, not the strip: explicit widths are not scaled to fill it + // (see normalizeColumnLayout), so a strip that underfills must end up against the RIGHT margin + // with the slack on the left — mirroring about the strip's own span would leave it pinned left + // and merely swap the columns inside it. Falls back to the span when the area is unknown, which + // is exact whenever the columns fill it (always so in equal mode). + const span = contentWidth ?? x; + return geometry.map((col) => ({ + ...col, + x: span - (col.x + col.width), + ...(col.separatorX === undefined ? {} : { separatorX: span - col.separatorX }), + })); } export function normalizeColumnLayout( @@ -165,8 +212,27 @@ export function normalizeColumnLayout( } // Per-column gaps drive geometry in explicit mode (step 4); equal mode uses the uniform gap. + // + // Clamped to >= 0 like the scalar `gap` above. OOXML cannot express a negative gutter — `w:space` + // is ST_TwipsMeasure, unsigned — and letting one through breaks the invariant the geometry API + // relies on: that in an LTR layout `x` rises with the column index. Direction-aware consumers read + // that monotonicity to tell a mirrored strip from an upright one, so a negative gap wide enough to + // pull a column back behind its predecessor would make an LTR layout answer hit tests as if it + // were RTL. + // A HOLE in the array falls back to the scalar, exactly as `buildColumnGeometry`'s `gaps?.[i] ?? + // gap` and `effectiveColumnGaps` already do for a SHORT one. `Math.max(0, undefined)` is NaN, and + // NaN is not nullish, so the geometry fallback below could not catch it: `{gap: 40, gaps: [30, + // undefined]}` normalized to `gaps: [30, NaN]` and painted `col2.x = NaN`, i.e. a column with no + // position at all. `gaps?: number[]` forbids a hole under TypeScript and nothing in this repo + // constructs a `gaps` array yet, so this is unreachable today — and it stops being unreachable the + // day the importer starts projecting `w:cols/w:col/@w:space` per column. A non-finite entry takes + // the same fallback, since the reason to distrust it is identical. const gaps = - explicitWidths.length > 0 && Array.isArray(input?.gaps) ? input.gaps.slice(0, Math.max(0, count - 1)) : undefined; + explicitWidths.length > 0 && Array.isArray(input?.gaps) + ? input.gaps + .slice(0, Math.max(0, count - 1)) + .map((value) => (Number.isFinite(value) ? Math.max(0, value) : Math.max(0, gap))) + : undefined; const width = widths.reduce((max, value) => Math.max(max, value), 0); @@ -176,6 +242,8 @@ export function normalizeColumnLayout( gap: 0, width: Math.max(0, contentWidth), ...(input?.withSeparator !== undefined ? { withSeparator: input.withSeparator } : {}), + ...(input?.direction !== undefined ? { direction: input.direction } : {}), + contentWidth: Math.max(0, contentWidth), }; } @@ -186,7 +254,9 @@ export function normalizeColumnLayout( ...(gaps && gaps.length > 0 ? { gaps } : {}), ...(input?.equalWidth !== undefined ? { equalWidth: input.equalWidth } : {}), ...(input?.withSeparator !== undefined ? { withSeparator: input.withSeparator } : {}), + ...(input?.direction !== undefined ? { direction: input.direction } : {}), width, + contentWidth: Math.max(0, contentWidth), }; } @@ -207,7 +277,14 @@ export function getColumnGeometry(normalized: NormalizedColumnLayout): ColumnGeo Array.isArray(normalized.widths) && normalized.widths.length > 0 ? normalized.widths : new Array(count).fill(normalized.width); - return buildColumnGeometry(widths, normalized.gap, Boolean(normalized.withSeparator), normalized.gaps); + return buildColumnGeometry( + widths, + normalized.gap, + Boolean(normalized.withSeparator), + normalized.gaps, + normalized.direction, + normalized.contentWidth, + ); } // --------------------------------------------------------------------------- @@ -242,13 +319,63 @@ export function getColumnSeparatorPositions(geometry: ColumnGeometry[], originX .map((col) => originX + (col.separatorX as number)); } -/** Index of the column containing absolute `x` (clicks in a gap map to the preceding column). */ +/** + * Index of the column whose OWN span contains absolute `x`, or `null` when `x` lies in no column at + * all — a gutter, the page margins, or something that is not column flow in the first place. + * + * This is the strict counterpart to `getColumnAtX` below, and the two exist because paint-time and + * hit-testing want opposite answers. A click has to select something, so `getColumnAtX` clamps and + * hands a gap to its neighbouring column. Asking "is there content in a later column" must not + * clamp: `page.items` carries page-anchored objects, and a full-width watermark belongs to no + * column, so answering with one makes it evidence for chrome Word does not draw. + * + * Direction-agnostic by construction. It tests containment in each column's own span instead of + * comparing against a boundary, so it does not care whether `x` ascends or descends with the index, + * and — unlike an edge test — it is not fooled by a fragment WIDER than its column. An over-wide + * table is placed at its column's left edge and overflows rightward in both directions, so its + * origin still identifies its column while its trailing edge does not. + * + * Spans are half-open — `[x, x + width)` — so that adjacent columns authored with no gutter at all + * (`w:space="0"`) do not both claim the boundary they share. That boundary is exactly where the + * later column's own content is placed, and an inclusive upper bound would hand it to the earlier + * column instead. Columns are scanned in fill order and the first containing span wins, which + * after that only matters for an overfull explicit strip whose columns genuinely overlap. + */ +export function findColumnContaining(geometry: ColumnGeometry[], x: number, originX = 0): number | null { + const cx = x - originX; + for (const col of geometry) { + if (cx >= col.x && cx < col.x + col.width) return col.index; + } + return null; +} + +/** + * Index of the column containing absolute `x` (clicks in a gap map to the preceding column). + * + * The walk is direction-aware and cannot assume ascending `x`: in an RTL section column 0 sits on + * the right, so `x` DESCENDS with the index. The mirrored branch keeps the same rule the LTR branch + * states — a point in a gap belongs to the column that precedes it in FILL order — which is what + * makes a drag that crosses the gutter keep extending from the column it is leaving instead of + * jumping. Direction is read off the geometry rather than taken as an argument, so every existing + * caller keeps working unchanged. + * + * Both branches test a HALF-OPEN span, so this agrees with `findColumnContaining` on every boundary + * the two can both answer. The LTR branch gets that from `cx >= col.x`: a point on a shared edge is + * the later column's, because that is where the later column's content begins. The mirrored branch + * has to say the same thing from the other side — the shared edge is the EARLIER fill column's left + * edge there — which is `cx < col.x + col.width`, exclusive. An inclusive bound handed that point to + * the later column, contradicting the half-open span the geometry places content in, and it also + * pulled in the point one pixel-width past a column's trailing edge, which is gutter and belongs to + * the preceding column. With `w:space="0"` the two coincide and every column boundary in an RTL + * section resolved one column too far. + */ export function getColumnAtX(geometry: ColumnGeometry[], x: number, originX = 0): number { if (geometry.length === 0) return 0; const cx = x - originX; + const mirrored = geometry.length > 1 && geometry[1].x < geometry[0].x; let result = 0; for (const col of geometry) { - if (cx >= col.x) result = col.index; + if (mirrored ? cx < col.x + col.width : cx >= col.x) result = col.index; else break; } return result; @@ -263,19 +390,29 @@ export function columnLayoutsEqual(a?: ColumnLayout, b?: ColumnLayout): boolean a.gap === b.gap && a.equalWidth === b.equalWidth && Boolean(a.withSeparator) === Boolean(b.withSeparator) && + (a.direction ?? 'ltr') === (b.direction ?? 'ltr') && widthsEqual(a.widths, b.widths) && widthsEqual(a.gaps, b.gaps) ); } +/** + * The gutters a layout will actually be drawn with: `gaps[i]` positionally when present, the scalar + * gap otherwise, each floored at 0. Mirrors `normalizeColumnLayout` so render equality stays exactly + * as discriminating as the geometry it stands in for. + */ +function effectiveColumnGaps(columns: ColumnLayout, count: number): number[] { + const gap = Math.max(0, columns.gap ?? 0); + const authored = Array.isArray(columns.gaps) ? columns.gaps : []; + return Array.from({ length: Math.max(0, count - 1) }, (_, i) => Math.max(0, authored[i] ?? gap)); +} + /** * Render equality: true when two column configs produce the SAME rendered layout even if their raw * fields differ. Compares the canonical render form for today's renderer (resolved mode + count, - * scalar gap, withSeparator, and in explicit mode the sliced widths) and deliberately ignores raw - * `equalWidth` and the surplus count/widths that resolution discards. Per-column `gaps` are - * intentionally ignored until geometry/separators consume them (step 4), so a gaps-only authored - * delta does not split regions or invalidate the normalized-columns cache before it becomes - * paint-significant. Use for region/cache change detection so e.g. `{num:4, widths:[a,b]}` vs + * scalar gap, withSeparator, and in explicit mode the sliced widths and per-column `gaps`) and + * deliberately ignores raw `equalWidth` and the surplus count/widths that resolution discards. + * Use for region/cache change detection so e.g. `{num:4, widths:[a,b]}` vs * `{num:2, widths:[a,b]}`, or `equalWidth:true` vs an omitted equalWidth, do not split into * separate regions. (SD-2629) */ @@ -285,12 +422,61 @@ export function columnRenderLayoutsEqual(a?: ColumnLayout, b?: ColumnLayout): bo const mode = resolveColumnMode(a); if (mode !== resolveColumnMode(b)) return false; if (resolveColumnCount(a) !== resolveColumnCount(b)) return false; - if ((a.gap ?? 0) !== (b.gap ?? 0)) return false; if (Boolean(a.withSeparator) !== Boolean(b.withSeparator)) return false; + // Direction IS paint-significant: it decides which side column 0 lands on, so two layouts that + // differ only here must split regions and invalidate the normalized-columns cache. + if ((a.direction ?? 'ltr') !== (b.direction ?? 'ltr')) return false; if (mode === 'explicit') { const ra = resolveColumnLayout(a); const rb = resolveColumnLayout(b); + // Per-column gaps ARE paint-significant. `buildColumnGeometry` reads `gaps[i] ?? gap` for both + // the column x and the separator x, so a gaps-only delta moves every column after the first. + // This comparison used to skip them, on the note that nothing consumed them yet; SD-2629 step 4 + // made that false, and while it was skipped two sections differing only in their per-column gaps + // compared equal — no region split, no cache invalidation, and the later section laid out with + // the earlier one's gutters. + // + // Compare the gutters that will actually be DRAWN, not the authored arrays. `resolveColumnLayout` + // emits `gaps` only when the author supplied them and pads a short array with 0, while geometry + // falls back to the scalar gap and floors at 0 — so the authored arrays are equal in cases that + // render differently (a short `[20]` vs `[20, 0]`) and differ in cases that render identically + // (an omitted array vs one spelling out the scalar gap, or a negative gap vs 0). Deriving the + // effective gutters the way `normalizeColumnLayout` does keeps this predicate exactly as + // discriminating as the geometry it is standing in for. + if (!widthsEqual(effectiveColumnGaps(a, resolveColumnCount(a)), effectiveColumnGaps(b, resolveColumnCount(b)))) { + return false; + } if (!widthsEqual(ra.widths, rb.widths)) return false; + // The one way the scalar still reaches explicit WIDTHS: `normalizeColumnLayout` floors a + // fabricated width at 1px, and collapses to a single column when the usable width falls to + // epsilon — both gated on the sign of `contentWidth - gap * (count - 1)`, which the scalar moves. + // It only bites when an authored width is itself sub-pixel (under ~15 twips), because at 1px or + // more the floor and the collapse are both no-ops. Rather than model a content-width-dependent + // branch in a predicate documented as content-width-INDEPENDENT, refuse to call such a pair equal + // at all: `{widths: [0.5, 0.5, 0.5], gaps: [1, 1]}` renders as three 1px columns under one scalar + // gap and three 0.5px columns under another, and at 1e-5 the second collapses to one full-width + // column. Costs nothing on any real document. + const hasSubPixelWidth = (resolved: ColumnLayout): boolean => (resolved.widths ?? []).some((w) => w < 1); + if ((hasSubPixelWidth(ra) || hasSubPixelWidth(rb)) && (a.gap ?? 0) !== (b.gap ?? 0)) return false; + } else if ((a.gap ?? 0) !== (b.gap ?? 0)) { + // Equal mode reads the scalar gap twice over — it IS every gutter, and `normalizeColumnLayout` + // subtracts the total from the content area before dividing it, so it sets the column width as + // well. Nothing else can stand in for it here. + // + // Explicit mode deliberately does NOT compare it on its own. There the scalar is only the + // fallback for a gutter `gaps` does not supply, and `effectiveColumnGaps` above already folds it + // in at exactly that position — so a layout whose `gaps` spell out every gutter renders + // identically no matter what the scalar says, and comparing it separately split a region and + // invalidated the normalized-columns cache over a value nothing read. The one route by which the + // scalar still reaches explicit WIDTHS — normalize's sub-pixel `Math.max(1, …)` floor and its + // epsilon collapse, both keyed on the sign of `contentWidth - gap * (count - 1)` — is handled by + // the `hasSubPixelWidth` guard in the explicit branch above, not waved off here: a width of 1px + // or more makes both no-ops, which is exactly the threshold that guard tests. That guard is + // COMPLETE rather than merely in scope — the epsilon collapse needs the MAXIMUM authored width + // at or under the epsilon, and both epsilons live in the tree are below 1px (1e-4 in + // `layout-engine/src/index.ts`, 1e-2 in `layout-bridge/src/incrementalLayout.ts`), so that route + // also implies a sub-pixel width. There is no path the guard misses. + return false; } return true; } diff --git a/packages/layout-engine/contracts/src/graphic-placement.ts b/packages/layout-engine/contracts/src/graphic-placement.ts index 744063713b..b80ead7eb6 100644 --- a/packages/layout-engine/contracts/src/graphic-placement.ts +++ b/packages/layout-engine/contracts/src/graphic-placement.ts @@ -1,4 +1,5 @@ import { getColumnGeometry, getColumnX } from './column-layout.js'; +import type { BaseDirection } from './direction-context.js'; /** ECMA-376 Part 1 §20.4.3.4 (`ST_RelFromH`). */ export const ANCHOR_H_RELATIVE_VALUES = [ @@ -105,6 +106,12 @@ export type ColumnLayoutForAnchor = { // stride; equal columns reduce to the old stride. (SD-2629) widths?: number[]; gaps?: number[]; + // Section page direction and the content width it was normalized against, both read by + // getColumnGeometry. Declared rather than left to structural pass-through: a column-relative + // anchor in an RTL section must resolve against the mirrored geometry, and silently dropping + // these would place it against the wrong margin with no type error to catch it. + direction?: BaseDirection; + contentWidth?: number; }; /** diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index e8b30134aa..aa4675ab18 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -21,6 +21,7 @@ export type { } from './direction-context.js'; export { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js'; import type { + BaseDirection, ParagraphDirectionContext, RunBidiContext, RunScriptContext, @@ -162,6 +163,7 @@ export { cloneColumnLayout, columnLayoutsEqual, columnRenderLayoutsEqual, + findColumnContaining, getColumnAtX, getColumnGapAfter, getColumnGeometry, @@ -2886,6 +2888,20 @@ export type ColumnLayout = { * mode uses the scalar `gap`. When absent, consumers fall back to the uniform `gap`. (SD-2629) */ gaps?: number[]; + /** + * Section page direction, from `w:sectPr/w:bidi`. Decides which side the FIRST column sits on: + * `'ltr'` (default) fills left to right, `'rtl'` fills right to left, matching Word. + * + * Per ECMA-376 §17.6.1 a section's `w:bidi` governs section-level chrome — page numbers, gutters + * and columns — and is independent of the paragraph inline direction (§17.3.1.6). It is carried + * here, on the column layout itself, because `getColumnGeometry` is the single source every + * column consumer reads for positioning (fill, hit testing, separators, balancing, floating + * anchors, footnotes); threading the axis alongside the widths keeps those consumers from having + * to re-derive it, and keeps them from disagreeing. + * + * Absent means `'ltr'`. Every existing producer therefore keeps its current geometry unchanged. + */ + direction?: BaseDirection; }; /** diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 7f4be68d14..8745d7ac63 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -1643,14 +1643,25 @@ const assignFootnotesToColumns = ( if (fragment?.kind === 'table' && typeof fragment.columnIndex === 'number') { columnIndex = Math.max(0, Math.min(columns.count - 1, fragment.columnIndex)); } else if (fragment && typeof fragment.x === 'number') { - // Geometry-derived midpoint assignment: assign the ref to the column whose right edge plus - // half its own gap the fragment falls before. Per-column widths/gaps come from the resolved + // Geometry-derived midpoint assignment: assign the ref to the column whose far edge plus + // half its own gap the fragment falls short of. Per-column widths/gaps come from the resolved // geometry, preserving the prior midpoint rule. The old uniform-stride branch was unreachable // for count>1 (normalized columns always carry widths). (SD-2629 4c) + // + // "Far edge" is direction-relative: in an RTL section column 0 sits on the right, so x + // DESCENDS with the index and the fragment must be compared against the column's LEFT edge + // minus half its gap instead. Walking the geometry with the LTR test in an RTL section + // matched column 0 for every fragment, which collapsed all of a page's footnotes into the + // first column's group — the left column's notes printed under the right column and its own + // note area stayed empty. const geometry = getColumnGeometry(columns); + const mirrored = geometry.length > 1 && geometry[1].x < geometry[0].x; columnIndex = Math.max(0, geometry.length - 1); for (const col of geometry) { - if (fragment.x < columns.left + col.x + col.width + col.gapAfter / 2) { + const boundary = mirrored + ? columns.left + col.x - col.gapAfter / 2 + : columns.left + col.x + col.width + col.gapAfter / 2; + if (mirrored ? fragment.x >= boundary : fragment.x < boundary) { columnIndex = col.index; break; } diff --git a/packages/layout-engine/layout-bridge/test/footnoteColumnPlacement.test.ts b/packages/layout-engine/layout-bridge/test/footnoteColumnPlacement.test.ts index 4fcb62854b..73dc0fc1b7 100644 --- a/packages/layout-engine/layout-bridge/test/footnoteColumnPlacement.test.ts +++ b/packages/layout-engine/layout-bridge/test/footnoteColumnPlacement.test.ts @@ -81,6 +81,69 @@ describe('Footnotes in columns', () => { expect(footnoteTwoFragment?.x).toBeCloseTo(columnTwoX, 2); }); + it('places footnotes in the mirrored column of their reference in an RTL section', async () => { + // Footnote refs are assigned to a column by comparing the reference fragment's x against each + // column's far edge plus half its gap. "Far edge" is direction-relative: in an RTL section + // column 0 sits on the right and x DESCENDS with the index, so the left-to-right test matches + // column 0 for every fragment and collapses the whole page's notes into the first column's + // group — the left column's notes print under the right column and its own note area is empty. + const paragraphOne = makeParagraph('para-1', 'Column 1 text', 0); + const columnBreak: FlowBlock = { kind: 'columnBreak', id: 'col-break-1' }; + const paragraphTwo = makeParagraph('para-2', 'Column 2 text', 40); + + const footnoteOne = makeParagraph('footnote-1-0-paragraph', 'Footnote one', 0); + const footnoteTwo = makeParagraph('footnote-2-0-paragraph', 'Footnote two', 0); + + const measureBlock = vi.fn(async (block: FlowBlock) => { + if (block.kind === 'columnBreak') { + return { kind: 'columnBreak' } as Measure; + } + const textLength = block.kind === 'paragraph' ? (block.runs?.[0]?.text?.length ?? 1) : 1; + const lineHeight = block.id.startsWith('footnote-') ? 10 : 18; + return makeMeasure(lineHeight, textLength); + }); + + const columns = { count: 2, gap: 20, direction: 'rtl' as const }; + const margins = { top: 60, right: 60, bottom: 60, left: 60 }; + const pageSize = { w: 600, h: 800 }; + + const result = await incrementalLayout( + [], + null, + [paragraphOne, columnBreak, paragraphTwo], + { + pageSize, + margins, + columns, + footnotes: { + refs: [ + { id: '1', pos: 2 }, + { id: '2', pos: 42 }, + ], + blocksById: new Map([ + ['1', [footnoteOne]], + ['2', [footnoteTwo]], + ]), + }, + }, + measureBlock, + ); + + const page = result.layout.pages[0]; + const columnWidth = (pageSize.w - margins.left - margins.right - columns.gap) / columns.count; + // Mirrored: fill column 0 is the RIGHT one, fill column 1 the left. + const firstColumnX = margins.left + columnWidth + columns.gap; + const secondColumnX = margins.left; + + const footnoteOneFragment = page.fragments.find((fragment) => fragment.blockId === footnoteOne.id); + const footnoteTwoFragment = page.fragments.find((fragment) => fragment.blockId === footnoteTwo.id); + + expect(footnoteOneFragment?.x).toBeCloseTo(firstColumnX, 2); + expect(footnoteTwoFragment?.x).toBeCloseTo(secondColumnX, 2); + // The two notes must land in DIFFERENT columns; collapsing them into one is the failure mode. + expect(footnoteOneFragment?.x).not.toBeCloseTo(footnoteTwoFragment?.x ?? 0, 2); + }); + it('keeps footnotes in the owning column for wide overflow tables', async () => { const paragraphOne = makeParagraph('para-1', 'Column 1 text', 0); const columnBreak: FlowBlock = { kind: 'columnBreak', id: 'col-break-1' }; diff --git a/packages/layout-engine/layout-bridge/test/position-hit.test.ts b/packages/layout-engine/layout-bridge/test/position-hit.test.ts index 421d93ccdb..80ecb0b749 100644 --- a/packages/layout-engine/layout-bridge/test/position-hit.test.ts +++ b/packages/layout-engine/layout-bridge/test/position-hit.test.ts @@ -111,6 +111,36 @@ describe('determineColumn (SD-2629: resolved per-column boundaries)', () => { expect(determineColumn(layout, 540, page)).toBe(2); }); + it('resolves a click to the visually containing column in an RTL section', () => { + // In an RTL section column 0 sits against the RIGHT margin, so a click on the right half of the + // page selects the FIRST column. Resolving this with the left-to-right rule sends every click to + // the wrong column — the issue's "clicks will select the wrong column". + const columns = { count: 3, gap: 24, direction: 'rtl' as const }; + const page = { + columns, + margins: { left: 96, right: 96 }, + size: { w: 816, h: 1056 }, + } as unknown as Page; + const layout = { pageSize: { w: 816, h: 1056 }, columns, pages: [page] } as unknown as Layout; + + // Content width 624 -> 192px columns with a 24px gutter between them. Mirrored, column 0 spans + // 528..720, column 1 312..504, column 2 96..288 (absolute) -- each start is the previous + // column's start less width+gap, so the gutters are 504..528 and 288..312. + expect(determineColumn(layout, 700, page)).toBe(0); + expect(determineColumn(layout, 400, page)).toBe(1); + expect(determineColumn(layout, 150, page)).toBe(2); + // The outer margins stay with their own end columns. + expect(determineColumn(layout, 816, page)).toBe(0); + expect(determineColumn(layout, 0, page)).toBe(2); + + // Same geometry without the direction keeps answering left to right. + const ltrColumns = { count: 3, gap: 24 }; + const ltrPage = { ...page, columns: ltrColumns } as unknown as Page; + const ltrLayout = { pageSize: { w: 816, h: 1056 }, columns: ltrColumns, pages: [ltrPage] } as unknown as Layout; + expect(determineColumn(ltrLayout, 700, ltrPage)).toBe(2); + expect(determineColumn(ltrLayout, 150, ltrPage)).toBe(0); + }); + it('maps a hit to its mid-page column region, not the page-start columns (SD-2629)', () => { // A continuous section break splits the page: region 0 (y 96-300) is single-column; region 1 // (y 300-700) is two-column. page.columns is only the page-START config (single column), so a 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 26b02b30e4..f9bcf36939 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.test.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.test.ts @@ -322,7 +322,28 @@ function createMeasure(kind: string, lineHeights: number[]): { kind: string; lin } describe('balanceSectionOnPage', () => { - type TestFragment = { blockId: string; x: number; y: number; width: number; kind: string }; + type TestFragment = { + blockId: string; + x: number; + y: number; + width: number; + kind: string; + columnIndex?: number; + height?: number; + }; + + /** + * Read the balanced page back the way a person does — each column top to bottom, columns in fill + * order — and return the document indices in that sequence. Asserting on this rather than on exact + * y values keeps the test about ORDER, so it does not break when the balancer legitimately picks a + * different split point. + */ + function readingOrder(fragments: TestFragment[], firstColumnX: number): number[] { + return fragments + .map((fragment, index) => ({ index, column: fragment.x === firstColumnX ? 0 : 1, y: fragment.y })) + .sort((a, b) => a.column - b.column || a.y - b.y) + .map((entry) => entry.index); + } /** Build a fragment + section mapping for section-scoped tests. */ function buildSectionFixture( @@ -347,6 +368,565 @@ describe('balanceSectionOnPage', () => { return { fragments, measureMap, blockSectionMap }; } + it('keeps an RTL section right-to-left on the balanced page', () => { + // Balancing REBUILDS the geometry and then overwrites every fragment's x from it, so a dropped + // direction does not fail loudly: the last page of a two-column Hebrew section would simply be + // laid out left-to-right while every earlier page of the same section was right-to-left. + 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, direction: 'rtl', contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // The FIRST three paragraphs land in the RIGHT column (x = left margin + 288 + 48), the last + // three in the left one — the mirror image of the LTR case above. + expect(fragments.slice(0, 3).map((f) => f.x)).toEqual([432, 432, 432]); + expect(fragments.slice(3).map((f) => f.x)).toEqual([96, 96, 96]); + }); + + it('reads an already-columnised RTL page in document order, not left to right', () => { + // Balancing re-derives document order from the fragments' current positions, because the + // paginator fills column 0 top-to-bottom before moving on. In an RTL section column 0 is the + // RIGHT one, so document order DESCENDS in x; ordering the page left-to-right would feed the + // balancer the trailing column first and scramble the reading order of the balanced page. + const top = 96; + const RIGHT = 432; // left margin 96 + column width 288 + gap 48 + const LEFT = 96; + // Paragraphs 0-3 were laid out in the right column, 4-5 spilled into the left one. + const placements: Array<{ x: number; y: number }> = [ + { x: RIGHT, y: top }, + { x: RIGHT, y: top + 20 }, + { x: RIGHT, y: top + 40 }, + { x: RIGHT, y: top + 60 }, + { x: LEFT, y: top }, + { x: LEFT, y: top + 20 }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ blockId: id, x: placement.x, y: placement.y, width: 288, kind: 'para' }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, direction: 'rtl', contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // 3+3 balance, still in document order: 0-2 in the right column, 3-5 in the left one. + expect(fragments.map((f) => f.x)).toEqual([RIGHT, RIGHT, RIGHT, LEFT, LEFT, LEFT]); + expect(fragments.map((f) => f.y)).toEqual([top, top + 20, top + 40, top, top + 20, top + 40]); + }); + + it('keeps document order when fragments in one column do not share an x', () => { + // Document order was reconstructed by sorting on raw x, on the premise that every fragment in a + // column starts at the same place. A negative w:ind, a float offset, or a right-aligned wide + // table all break that premise, and the balanced x/y are written back in whatever order the sort + // produced — so the page is REORDERED, not merely laid out oddly. + const top = 96; + const LEFT = 96; + const RIGHT = 432; // margin 96 + column 288 + gap 48 + // Paragraphs 0-3 fill the left column; #1 is indented 20px and #2 by a hair. 4-5 spilled right. + const placements = [ + { x: LEFT, y: top }, + { x: LEFT + 20, y: top + 20 }, + { x: LEFT + 1e-7, y: top + 40 }, + { x: LEFT, y: top + 60 }, + { x: RIGHT, y: top }, + { x: RIGHT, y: top + 20 }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ blockId: id, x: placement.x, y: placement.y, width: 288, kind: 'para' }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // Reading the page back gives document order. Sorting on x would have put #1 (x = 116) after + // #0, #2 and #3, so paragraph 1 would surface in the wrong place entirely. + expect(readingOrder(fragments, LEFT)).toEqual([0, 1, 2, 3, 4, 5]); + // Both columns are actually used, or the assertion above proves nothing. + expect(new Set(fragments.map((f) => f.x)).size).toBe(2); + }); + + it('keeps document order for an over-wide table whose x sits outside its column', () => { + // resolveTableFrame right-aligns an over-wide table inside its column, which puts its origin at + // a NEGATIVE offset — outside the column, and left of every fragment in the column before it. + const top = 96; + const LEFT = 96; + const RIGHT = 432; + const placements: TestFragment[] = [ + { blockId: '', x: LEFT, y: top, width: 288, kind: 'para' }, + { blockId: '', x: LEFT, y: top + 20, width: 288, kind: 'para' }, + { blockId: '', x: LEFT, y: top + 40, width: 288, kind: 'para' }, + // In the SECOND column, but right-aligned and 500 wide, so resolveTableFrame gives it + // 432 + (288 - 500) = 220 — an origin that lands INSIDE the first column's span. Nothing about + // its box says which column owns it; only the columnIndex the engine recorded does. + { blockId: '', x: 220, y: top, width: 500, kind: 'table', columnIndex: 1 }, + { blockId: '', x: RIGHT, y: top + 20, width: 288, kind: 'para' }, + { blockId: '', x: RIGHT, y: top + 40, width: 288, kind: 'para' }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ ...placement, blockId: id }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // It keeps its place in reading order. Sorting on x would have pulled it forward, because 220 is + // lower than every other fragment's x on the page. + expect(readingOrder(fragments, LEFT)).toEqual([0, 1, 2, 3, 4, 5]); + expect(new Set(fragments.map((f) => f.x)).size).toBe(2); + }); + + it('keeps document order for an over-wide anchored table by the column it recorded', () => { + // Same shape as the test above, reached the other way: an ANCHORED table. + // `createAnchoredTableFragment` used to record no column at all, which left a floating over-wide + // table to be placed from its box alone — and its box cannot say which column owns it, because + // resolveTableFrame's `end` placement (the default for any w:bidiVisual table) puts its origin + // inside an EARLIER column while its trailing edge sits on its own. The factory now writes + // `state.columnIndex`, so the record answers it. + const top = 96; + const LEFT = 96; + const RIGHT = 432; + const placements: TestFragment[] = [ + { blockId: '', x: LEFT, y: top, width: 288, kind: 'para' }, + { blockId: '', x: LEFT, y: top + 20, width: 288, kind: 'para' }, + { blockId: '', x: LEFT, y: top + 40, width: 288, kind: 'para' }, + // 432 + (288 - 500) = 220, an origin that lands inside the FIRST column's span. + { blockId: '', x: 220, y: top, width: 500, kind: 'table', columnIndex: 1 }, + { blockId: '', x: RIGHT, y: top + 20, width: 288, kind: 'para' }, + { blockId: '', x: RIGHT, y: top + 40, width: 288, kind: 'para' }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ ...placement, blockId: id }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // Without the record, containment on the origin puts it in column 0 and hoists it above the two + // paragraphs that precede it there. + expect(readingOrder(fragments, LEFT)).toEqual([0, 1, 2, 3, 4, 5]); + expect(new Set(fragments.map((f) => f.x)).size).toBe(2); + }); + + it('updates a recorded column when balancing moves the fragment to another one', () => { + // The canonical last page of a balanced section: the paginator filled column 0 top to bottom and + // left column 1 empty, so everything it recorded a column for recorded column 0. Balancing then + // moves the tail into column 1 — and the record has to follow, because two consumers now treat + // it as authoritative. Ordering above prefers it over any geometry, and the DOM painter's + // separator gate reads it to decide which columns hold content, so a table left claiming column + // 0 while painted in column 1 can suppress a separator Word draws. + const top = 96; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + for (let i = 0; i < 5; i += 1) { + const id = `p${i}`; + fragments.push({ blockId: id, x: 96, y: top + i * 20, width: 288, kind: 'para' }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 0); + } + // layout-table.ts stamps columnIndex on a flow table; this one was laid out in column 0. + fragments.push({ blockId: 'tbl', x: 96, y: top + 100, width: 288, kind: 'table', height: 20, columnIndex: 0 }); + measureMap.set('tbl', createMeasure('table', [])); + blockSectionMap.set('tbl', 0); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 0, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + const table = fragments.find((f) => f.blockId === 'tbl')!; + expect(table.x).toBe(432); + expect(table.columnIndex).toBe(1); + // A paragraph the paginator recorded nothing for still records nothing: the presence of a value + // is the engine's own evidence of ownership, and the separator gate reads it as such. + expect(fragments.filter((f) => f.blockId !== 'tbl').every((f) => f.columnIndex === undefined)).toBe(true); + }); + + describe('resolving which column owns a fragment', () => { + // Every case here uses EQUAL columns, because `hasGenuinelyUnequalExplicitColumnWidths` makes + // balancing skip a genuinely unequal explicit section outright — an unequal-width fixture would + // return null and prove nothing. + // + // 200px columns over a 420px content area with a 20px gutter: column 0 spans 96..296 absolute, + // column 1 spans 316..516. Each case builds four 20px fragments, two per column, and replaces + // one of them with the fragment under test. Correct ordinals always read back as [0, 1, 2, 3]; + // balancing splits the page 2/2, so a misresolved ordinal hoists the fragment past a sibling and + // shows up as a swap rather than merely an odd position. + const TOP = 96; + const LEFT = 96; + const COL1 = 316; + + function orderWith(slot: number, special: TestFragment): number[] { + const base: TestFragment[] = [ + { blockId: '', x: LEFT, y: TOP, width: 200, kind: 'para' }, + { blockId: '', x: LEFT, y: TOP + 20, width: 200, kind: 'para' }, + { blockId: '', x: COL1, y: TOP, width: 200, kind: 'para' }, + { blockId: '', x: COL1, y: TOP + 20, width: 200, kind: 'para' }, + ]; + base[slot] = { ...special, y: base[slot].y }; + + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + base.forEach((fragment, i) => { + const id = `s2-b${i}`; + fragments.push({ ...fragment, blockId: id }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 20, width: 200, contentWidth: 420 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: LEFT }, + topMargin: TOP, + columnWidth: 200, + availableHeight: 60, + measureMap, + }); + expect(result).not.toBeNull(); + return readingOrder(fragments, LEFT); + } + + it('takes the column the engine recorded over anything the geometry says', () => { + // A footnote body is placed in its own note band, so its x can name one column while the flow + // column that owns it is another (contracts/index.ts calls the field "distinct from visual + // x"). Here the box sits exactly on column 0's leading edge — the strongest signal geometry + // has — while the record says column 1. The record has to win, or a note band's ordering comes + // from where it was drawn rather than what it belongs to. + expect(orderWith(2, { blockId: '', x: LEFT, y: 0, width: 200, kind: 'para', columnIndex: 1 })).toEqual([ + 0, 1, 2, 3, + ]); + }); + + it('reads a table as wide as the whole content area from its own column', () => { + // A table at column 0's left edge spanning the entire content area. Its origin IS column 0's + // origin, so containment answers it without needing to reason about the overhang at all. + expect(orderWith(1, { blockId: '', x: LEFT, y: 0, width: 420, kind: 'table', height: 20 })).toEqual([0, 1, 2, 3]); + }); + + it('resolves an over-wide table from its record, because its box cannot', () => { + // These two boxes are THE SAME box. resolveTableFrame places a 600px `end`-justified table in + // column 1 at 316 + (200 - 600) = -84, spanning content-relative -180..420. A 400px table + // centred in column 1 sits at 216, spanning 120..420. Both end on column 1's trailing edge; + // both begin inside column 0; and for the pair below they even overlap the two columns + // identically. Nothing about either box distinguishes it from a box of the same shape whose + // owner is column 0 — see the content-area-wide case further down, which really does belong to + // column 0 while ending on a later column's trailing edge. Only the record settles it. + expect( + orderWith(2, { blockId: '', x: -84, y: 0, width: 600, kind: 'table', height: 20, columnIndex: 1 }), + ).toEqual([0, 1, 2, 3]); + expect( + orderWith(2, { blockId: '', x: 216, y: 0, width: 400, kind: 'table', height: 20, columnIndex: 1 }), + ).toEqual([0, 1, 2, 3]); + }); + + it('reads an outdented paragraph from the column it covers, not the one its origin fell into', () => { + // A negative `w:ind w:left` moves the origin OUT of its column and widens the fragment by the + // same amount. An outdent larger than the gutter therefore lands the origin inside the + // PREVIOUS column: a 50px outdent in column 1 gives x 266 (316 - 50) with width 250 + // (200 + 50), and 266 is inside column 0's span of 96..296. Containment alone names column 0 + // and pulls the paragraph ahead of column 0's own content. + // + // This is what the width gate is for. 250 does not fit column 0's 200, so the origin is not + // evidence of ownership, and the overlap vote decides: 30px of column 0 against the whole + // 200px of column 1. + expect(orderWith(2, { blockId: '', x: 266, y: 0, width: 250, kind: 'para' })).toEqual([0, 1, 2, 3]); + }); + + it('keeps a right-aligned float paragraph in the column its origin sits in', () => { + // `layout-paragraph.ts` re-points a `floatAlignment: 'right'` fragment at + // `columnX + (columnWidth - maxLineWidth)` and does NOT reduce its width, so a 50px line in a + // 200px column yields x 246 with width still 200 — an origin inside its own column and a right + // edge 150px past it. This is why the origin must NOT be gated on the box fitting its column: + // gating it sent this to an overlap vote, which sees 50px in column 0 against 130px in column + // 1 and moves the paragraph to the end of the page. + expect(orderWith(1, { blockId: '', x: LEFT + 150, y: 0, width: 200, kind: 'para' })).toEqual([0, 1, 2, 3]); + }); + }); + + it('still reads an indented fragment from the column its origin sits in', () => { + // The trailing-edge rule must not swallow the case it was added beside. A paragraph indented by + // a `w:ind` matches NEITHER column edge, and it is the origin that identifies it — so long as + // the box still fits the column it starts in, which is what an indent leaves behind. + const top = 96; + const LEFT = 96; + const RIGHT = 432; + const placements: TestFragment[] = [ + { blockId: '', x: LEFT, y: top, width: 288, kind: 'para' }, + // A real `w:ind w:left` of 36px: the origin moves in by 36 and the width comes DOWN by 36, so + // the fragment lands on neither column edge and the origin is all there is to go on. + { blockId: '', x: LEFT + 36, y: top + 20, width: 252, kind: 'para' }, + { blockId: '', x: LEFT, y: top + 40, width: 288, kind: 'para' }, + // The same indent in column 1. + { blockId: '', x: RIGHT + 36, y: top, width: 252, kind: 'para' }, + { blockId: '', x: RIGHT, y: top + 20, width: 288, kind: 'para' }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ ...placement, blockId: id }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + expect(readingOrder(fragments, LEFT)).toEqual([0, 1, 2, 3, 4]); + }); + + it('keeps a content-area-wide centred box in the column it was centred in', () => { + // The counter-example that makes a trailing-edge rule unusable. Three 192px columns over a 624px + // content area with 24px gutters sit at 0 / 216 / 432. A box the full width of the content area, + // centred in column 0, is placed at 0 + (192 - 624) / 2 = -216 and so spans -216..408 — and 408 + // is EXACTLY column 1's trailing edge (216 + 192). Reading the trailing edge therefore hands a + // column-0 box to column 1. It is not a coincidence of these numbers: for a content-area-wide box + // centred in column 0 the right edge is (columnWidth + contentWidth) / 2, which lands on the + // middle column's trailing edge for every odd column count. + // + // The origin is outside every column here, so the overlap vote decides, and the box covers + // column 0 and column 1 equally — 192px each — which the vote breaks toward the earlier column. + const top = 96; + const COLS = [96, 312, 528]; + const placements: TestFragment[] = [ + { blockId: '', x: COLS[0], y: top, width: 192, kind: 'para' }, + { blockId: '', x: -120, y: top + 20, width: 624, kind: 'table', height: 20 }, + { blockId: '', x: COLS[1], y: top, width: 192, kind: 'para' }, + { blockId: '', x: COLS[1], y: top + 20, width: 192, kind: 'para' }, + { blockId: '', x: COLS[2], y: top, width: 192, kind: 'para' }, + { blockId: '', x: COLS[2], y: top + 20, width: 192, kind: 'para' }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ ...placement, blockId: id }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 3, gap: 24, width: 192, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 192, + availableHeight: 40, + measureMap, + }); + + expect(result).not.toBeNull(); + // Three columns, so read the page by ascending x then y rather than through the two-column + // helper above. + const reading = fragments + .map((fragment, index) => ({ index, x: fragment.x, y: fragment.y })) + .sort((a, b) => a.x - b.x || a.y - b.y) + .map((entry) => entry.index); + expect(reading).toEqual([0, 1, 2, 3, 4, 5]); + }); + + it('recovers document order from a shuffled fragment array', () => { + // The paginator's array order is not a contract, so the ordinal/y ordering has to stand on its + // own rather than leaning on the stability of an already-sorted input. + const top = 96; + const LEFT = 96; + const RIGHT = 432; + const byIndex = [ + { x: LEFT, y: top }, + { x: LEFT, y: top + 20 }, + { x: LEFT, y: top + 40 }, + { x: RIGHT, y: top }, + { x: RIGHT, y: top + 20 }, + { x: RIGHT, y: top + 40 }, + ]; + const shuffled = [3, 0, 5, 2, 4, 1]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + shuffled.forEach((documentIndex) => { + const id = `s2-b${documentIndex}`; + fragments.push({ blockId: id, ...byIndex[documentIndex], width: 288, kind: 'para' }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: 96 }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + // Read the page back and recover the document indices, which the blockIds carry. + const recovered = fragments + .map((fragment) => ({ + n: Number(fragment.blockId.slice('s2-b'.length)), + column: fragment.x === LEFT ? 0 : 1, + y: fragment.y, + })) + .sort((a, b) => a.column - b.column || a.y - b.y) + .map((entry) => entry.n); + expect(recovered).toEqual([0, 1, 2, 3, 4, 5]); + }); + + it('resolves columns against the page margin, not the page origin', () => { + // Fragment x is absolute; column geometry is content-relative. A left margin wide enough to be + // mistaken for a column offset catches the conversion being dropped. + const top = 96; + const MARGIN = 300; + const LEFT = MARGIN; // column 0 + const RIGHT = MARGIN + 288 + 48; // column 1 + const placements = [ + { x: LEFT, y: top }, + { x: LEFT, y: top + 20 }, + { x: LEFT, y: top + 40 }, + { x: RIGHT, y: top }, + { x: RIGHT, y: top + 20 }, + { x: RIGHT, y: top + 40 }, + ]; + const fragments: TestFragment[] = []; + const measureMap = new Map }>(); + const blockSectionMap = new Map(); + placements.forEach((placement, i) => { + const id = `s2-b${i}`; + fragments.push({ blockId: id, x: placement.x, y: placement.y, width: 288, kind: 'para' }); + measureMap.set(id, createMeasure('paragraph', [20])); + blockSectionMap.set(id, 2); + }); + + const result = balanceSectionOnPage({ + fragments, + sectionIndex: 2, + sectionColumns: { count: 2, gap: 48, width: 288, contentWidth: 624 }, + sectionHasExplicitColumnBreak: false, + blockSectionMap, + margins: { left: MARGIN }, + topMargin: top, + columnWidth: 288, + availableHeight: 60, + measureMap, + }); + + expect(result).not.toBeNull(); + expect(readingOrder(fragments, LEFT)).toEqual([0, 1, 2, 3, 4, 5]); + expect(new Set(fragments.map((f) => f.x)).size).toBe(2); + }); + it('balances the target section and returns the tallest balanced column bottom', () => { // 6 equal paragraphs in a 2-col section → 3+3 balanced, tallest col ends at top + 3×20 = top + 60. const top = 96; @@ -619,6 +1199,43 @@ describe('balanceSectionOnPage', () => { ...extra, }); + it('moves the recorded column of a split half to the column it lands in', () => { + // Column ordering trusts a fragment's own `columnIndex` ahead of any geometry, so a record + // that contradicts the placement is worse than none. The spread that builds the second half + // copies the FIRST half's column, and the half is then placed one column further on — so a + // later balancing pass would read the stale record and sort the half back into the column it + // was moved out of, undoing the split it is the other half of. + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + // The paginator records the column for a footnote body and for tables, not for an ordinary + // paragraph; C carries one here to stand for the fragments that do. + const c = fragments.find((f) => f.blockId === 'C')!; + c.columnIndex = 0; + + expect(balance(fragments, measureMap, blockSectionMap)).not.toBeNull(); + + const [c1, c2] = (fragments.filter((f) => f.blockId === 'C') as SplitFragment[]).sort( + (a, b) => (a.fromLine ?? 0) - (b.fromLine ?? 0), + ); + expect(c1.x).toBe(96); + expect(c2.x).toBe(COL1_X); + // Each half's record now names the column it is actually in. + expect(c1.columnIndex).toBe(0); + expect(c2.columnIndex).toBe(1); + }); + + it('does not invent a recorded column for a half that never had one', () => { + // The separator gate reads the same field and treats a value as the engine's own evidence of + // ownership, so filling it in for a paragraph that carried nothing would be asserting more + // than the paginator ever knew. + const { fragments, measureMap, blockSectionMap } = straddleFixture(); + + expect(balance(fragments, measureMap, blockSectionMap)).not.toBeNull(); + + const halves = fragments.filter((f) => f.blockId === 'C') as SplitFragment[]; + expect(halves.length).toBe(2); + expect(halves.every((half) => half.columnIndex === undefined)).toBe(true); + }); + it('splits a straddling paragraph at a line boundary so columns balance', () => { const { fragments, measureMap, blockSectionMap } = straddleFixture(); diff --git a/packages/layout-engine/layout-engine/src/column-balancing.ts b/packages/layout-engine/layout-engine/src/column-balancing.ts index 7a5dcee3b9..3dbb898653 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.ts @@ -6,7 +6,14 @@ * matching Microsoft Word's behavior. */ -import { getColumnGeometry, getColumnX, hasGenuinelyUnequalExplicitColumnWidths } from '@superdoc/contracts'; +import { + findColumnContaining, + getColumnAtX, + getColumnGeometry, + getColumnX, + hasGenuinelyUnequalExplicitColumnWidths, +} from '@superdoc/contracts'; +import type { BaseDirection } from '@superdoc/contracts'; // ============================================================================ // Types and Interfaces @@ -657,6 +664,16 @@ export interface SectionColumnLayout { */ gaps?: number[]; equalWidth?: boolean; + /** + * Section page direction (`w:sectPr/w:bidi`) and the content width it was normalized against. + * + * Declared here — and not left to the structural subset above — because balancing REBUILDS the + * geometry and then overwrites every fragment's `x` from it. A balanced page that dropped these + * would be laid out left-to-right while every earlier page of the same RTL section was laid out + * right-to-left, so the last page of a two-column Hebrew section would visibly flip. + */ + direction?: BaseDirection; + contentWidth?: number; } export interface BalanceSectionOnPageArgs { @@ -783,13 +800,123 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu precedingHeight: precedingHeightBeforeTable, }); - // Order fragments in document order: by current column (x → left-to-right), - // then by y within each column. During unbalanced layout the paginator fills - // column 0 top-to-bottom, then column 1, etc. — so (x, y) preserves the - // original sequence. + // Order fragments in document order: by the column each one occupies, then by y within it. During + // unbalanced layout the paginator fills column 0 top-to-bottom, then column 1, etc., so column + // ordinal followed by y reproduces the original sequence — and the balanced x/y are written back + // onto the fragments in exactly this order, so getting it wrong reorders the page rather than + // just laying it out oddly. + // + // Resolve the ordinal from the fragment rather than sorting on raw `x`. Sorting on x assumes every + // fragment in a column shares one origin, which is false for a negative `w:ind`, a float offset, + // or a right-aligned over-wide table — and a difference of 1e-7 is enough to swap two paragraphs. + // The ordinal is also direction-free: it is the fill order, so it needs no RTL special case, where + // an x comparison needs one because column 0 sits on the right and document order descends in x. + const preBalanceGeometry = getColumnGeometry({ + count: columnCount, + gap: columnGap, + width: columnWidth, + ...(Array.isArray(sectionColumns.widths) ? { widths: sectionColumns.widths } : {}), + ...(Array.isArray(sectionColumns.gaps) ? { gaps: sectionColumns.gaps } : {}), + ...(sectionColumns.direction !== undefined ? { direction: sectionColumns.direction } : {}), + ...(sectionColumns.contentWidth !== undefined ? { contentWidth: sectionColumns.contentWidth } : {}), + }); + const originX = args.margins.left; + const clampOrdinal = (value: number): number => Math.max(0, Math.min(columnCount - 1, Math.floor(value))); + // Tolerance for "this box is no wider than that column". Widths reach fragments as the column + // width itself, so the slack only absorbs float drift (measured worst case ~5e-13px across + // awkward page/margin/count combinations). One twip is 96/1440 = 0.067px, so this stays well + // under the smallest width difference a document can express. + const WIDTH_EPSILON = 0.01; + + /** + * Which column a fragment occupies, as a fill-order ordinal. ALWAYS a number: the result is a sort + * key, and a key that is sometimes absent would leave the comparator mixing two different metrics, + * which is not a total order — `Array.prototype.sort` is then free to return different orders for + * the same input, and it does differ between engines. + * + * Geometry alone CANNOT settle this for a box wider than its column, and it is worth being exact + * about why, because every plausible repair trades one wrong answer for another. `resolveTableFrame` + * places an over-wide table that justifies to `end` at `col.x + (col.width - width)`, so the box + * runs from an earlier column's left edge to its OWN column's right edge — and the owner is the + * LATER column. A box of identical shape centred in column 0, an anchored graphic sized to the + * content area, runs between exactly those same two edges — and the owner is the EARLIER column. + * Same span, same per-column overlaps, opposite owners. The origin, the trailing edge and the + * overlap vote are each provably unable to tell those two apart, so the only thing that can is + * what the engine recorded: `columnIndex` is asked first, and is now written for anchored tables + * as well as in-flow ones. + * + * For everything else the question is only whether the origin can be trusted, and the test that + * answers it is the box's WIDTH, not its edges. A box no wider than the column its origin sits in + * was placed inside that column, wherever in it the origin ended up — ordinary content, a + * `w:ind` indent, and a `floatAlignment` right/centre paragraph, which `layout-paragraph.ts` + * re-points at `columnX + (columnWidth - maxLineWidth)` while leaving its width at the full column + * width. A box WIDER than that column may instead have been pulled left out of its own column: a + * negative `w:ind` widens the fragment by the outdent, so an outdent bigger than the gutter lands + * the origin inside the PREVIOUS column and containment names that one. + * + * Gating on the box's right EDGE staying inside the column instead of on its width was tried and + * was worse: it rejects the right-aligned float too, and the overlap vote then hands a short + * right-aligned line to whichever neighbour its overhang covers more of, reordering the page. + */ + const ordinalOf = (fragment: BalancingFragment): number => { + // The engine's own record, where it kept one. Only a few fragment kinds do: tables + // (`layout-table.ts`, five sites, plus `createAnchoredTableFragment` now), footnote bodies, and a + // paragraph ONLY when it is a collapsed split-line-break anchor carrier (`layout-paragraph.ts`, + // under `collapseSplitLineBreakCarrier`) — a narrow document shape, not the ordinary paragraph. + // The rule below therefore still carries almost every fragment. + const recorded = (fragment as { columnIndex?: number }).columnIndex; + if (typeof recorded === 'number' && Number.isFinite(recorded)) return clampOrdinal(recorded); + + const span = Number.isFinite(fragment.width) && fragment.width > 0 ? fragment.width : 0; + const left = fragment.x - originX; + + // The column the origin sits in, so long as the box is narrow enough to have been placed there. + const byOrigin = findColumnContaining(preBalanceGeometry, fragment.x, originX); + if (byOrigin !== null) { + const originColumn = preBalanceGeometry.find((col) => col.index === byOrigin); + if (originColumn && span <= originColumn.width + WIDTH_EPSILON) return byOrigin; + } + + // Either the origin sits in no column at all — an outdent or float offset smaller than the + // gutter hung it in there — or the box is too wide for the column the origin landed in, so that + // origin is not evidence of ownership. It still lies mostly in the column that owns it. Ties go + // to the earliest column in fill order, but only approximately: two mathematically equal + // overlaps derived from a content width that does not divide evenly can differ in their last + // bits and settle either way. + let best: number | null = null; + let bestOverlap = 0; + for (const col of preBalanceGeometry) { + const overlap = Math.min(left + span, col.x + col.width) - Math.max(left, col.x); + if (overlap > bestOverlap) { + bestOverlap = overlap; + best = col.index; + } + } + // Touching no column at all (zero width, wholly inside a gutter, or off the strip): fall back to + // the hit-testing walk, which clamps and therefore always names one. + return best ?? clampOrdinal(getColumnAtX(preBalanceGeometry, fragment.x, originX)); + }; + + // Order fragments in document order: by the column each one occupies, then by y within it. During + // unbalanced layout the paginator fills column 0 top-to-bottom, then column 1, etc., so column + // ordinal followed by y reproduces the original sequence — and the balanced x/y are written back + // onto the fragments in exactly this order, so getting it wrong REORDERS the page rather than + // merely laying it out oddly. + // + // Sorting on raw x, as this did, assumes every fragment in a column shares one origin. A negative + // `w:ind`, a float offset, or a right-aligned over-wide table each break that, and a difference of + // 1e-7 was enough to swap two paragraphs. The ordinal is also direction-free — it IS the fill + // order — where an x comparison needed an RTL special case because column 0 sits on the right. + const ordinals = new Map(); + sectionFragments.forEach((fragment) => ordinals.set(fragment, ordinalOf(fragment))); + const inputOrder = new Map(); + sectionFragments.forEach((fragment, index) => inputOrder.set(fragment, index)); const ordered = [...sectionFragments].sort((a, b) => { - if (a.x !== b.x) return a.x - b.x; - return a.y - b.y; + const byColumn = (ordinals.get(a) ?? 0) - (ordinals.get(b) ?? 0); + if (byColumn !== 0) return byColumn; + if (a.y !== b.y) return a.y - b.y; + // Same column, same y: keep the order they arrived in, so the sort is total and stable. + return (inputOrder.get(a) ?? 0) - (inputOrder.get(b) ?? 0); }); // Treat each fragment as its own block for binary-search balancing. Grouping @@ -864,6 +991,8 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu width: columnWidth, ...(Array.isArray(sectionColumns.widths) ? { widths: sectionColumns.widths } : {}), ...(Array.isArray(sectionColumns.gaps) ? { gaps: sectionColumns.gaps } : {}), + ...(sectionColumns.direction !== undefined ? { direction: sectionColumns.direction } : {}), + ...(sectionColumns.contentWidth !== undefined ? { contentWidth: sectionColumns.contentWidth } : {}), }); const columnX = (columnIndex: number): number => getColumnX(balancedGeometry, columnIndex, args.margins.left); @@ -876,6 +1005,18 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu f.x = columnX(col); f.y = colCursors[col]; f.width = columnWidth; + // Balancing MOVES fragments between columns, so a `columnIndex` the paginator wrote before the + // move now names the column the fragment left. Three consumers trust that record ahead of any + // geometry — `ordinalOf` above, the painter's separator gate, and `determineTableColumn` for hit + // testing — so a stale one is worse than none: an in-flow table balanced out of column 0 kept + // reporting column 0, which left its new column reading as empty and suppressed a rule Word + // draws. Rewrite it where the fragment had one; do not invent one where it did not, for the same + // reason the split branch below does not — an invented value is evidence the fragment never + // carried, and the gate would act on it. + const recordedColumn = (f as { columnIndex?: number }).columnIndex; + if (typeof recordedColumn === 'number' && Number.isFinite(recordedColumn)) { + (f as { columnIndex?: number }).columnIndex = col; + } // 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 @@ -900,6 +1041,15 @@ export function balanceSectionOnPage(args: BalanceSectionOnPageArgs): { maxY: nu continuesFromPrev: true, continuesOnNext: originalContinuesOnNext, } as BalancingFragment; + // The half lands in the NEXT column, so a `columnIndex` carried over by the spread would still + // name the FIRST half's column — and column ordering trusts that record ahead of any geometry, + // so a later balancing pass would sort this half back into the column it left. Rewrite it + // where the source had one; do not invent one where it did not, because the separator gate + // reads the same field and an invented value is evidence it never had. + const carriedColumn = (f as { columnIndex?: number }).columnIndex; + if (typeof carriedColumn === 'number' && Number.isFinite(carriedColumn)) { + (secondHalf as { columnIndex?: number }).columnIndex = col + 1; + } // 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. diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index ae5fd6ed12..6e8a6d0cf3 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -7315,7 +7315,7 @@ describe('requirePageBoundary edge cases', () => { expect(right.width).toBeCloseTo(365.4); }); - it('keeps the current explicit column after a manual column break when only later per-column gaps differ', () => { + it('opens a new region after a manual column break when later per-column gaps differ', () => { const toExplicitColumns: FlowBlock = { kind: 'sectionBreak', id: 'sb-explicit', @@ -7366,15 +7366,17 @@ describe('requirePageBoundary edge cases', () => { expect(p2.x).toBeCloseTo(50); expect(p3.x).toBeCloseTo(expectedSecondColumnX); - expect(p4.x).toBeCloseTo(expectedSecondColumnX); - expect(page.columnRegions).toHaveLength(2); - expect(page.columnRegions?.[1]?.columns).toEqual({ + // Per-column gaps drive geometry, so the delta is paint-significant and opens its own region + // carrying the NEW gaps. It used to compare equal, and p4 was laid out with the old gutters. + expect(page.columnRegions).toHaveLength(3); + expect(page.columnRegions?.at(-1)?.columns).toEqual({ count: 3, gap: 48, widths: [100, 100, 300], - gaps: [48, 48], + gaps: [48, 96], equalWidth: false, }); + expect(p4.x).toBeCloseTo(50); }); it('does not balance the final page for explicit custom-width columns', () => { diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index bc4ec340cf..73935ec5da 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -3920,7 +3920,9 @@ function* layoutDocumentSteps( const anchorX = tableBlock.anchor?.offsetH ?? columnX(state); floatManager.registerTable(tableBlock, tableMeasure, anchorY, state.columnIndex, state.page.number); - state.page.fragments.push(createAnchoredTableFragment(tableBlock, tableMeasure, anchorX, anchorY)); + state.page.fragments.push( + createAnchoredTableFragment(tableBlock, tableMeasure, anchorX, anchorY, state.columnIndex), + ); } } @@ -5193,6 +5195,11 @@ function toBalancingColumns(normalized: NormalizedColumns): SectionColumnLayout ...(Array.isArray(normalized.widths) ? { widths: normalized.widths } : {}), ...(Array.isArray(normalized.gaps) ? { gaps: normalized.gaps } : {}), ...(normalized.equalWidth !== undefined ? { equalWidth: normalized.equalWidth } : {}), + // Direction and the content width it was measured against travel with the widths: balancing + // rebuilds the geometry and overwrites fragment x from it, so dropping them here would lay the + // balanced page out left-to-right inside an otherwise right-to-left section. + ...(normalized.direction !== undefined ? { direction: normalized.direction } : {}), + ...(normalized.contentWidth !== undefined ? { contentWidth: normalized.contentWidth } : {}), }; } diff --git a/packages/layout-engine/layout-engine/src/layout-paragraph.ts b/packages/layout-engine/layout-engine/src/layout-paragraph.ts index 1115b1f972..b7f8c4dd9a 100644 --- a/packages/layout-engine/layout-engine/src/layout-paragraph.ts +++ b/packages/layout-engine/layout-engine/src/layout-paragraph.ts @@ -19,6 +19,7 @@ import type { TableAnchor, TableWrap, ParagraphLineRegion, + ColumnLayoutForAnchor, } from '@superdoc/contracts'; import { computeFragmentPmRange, @@ -469,7 +470,9 @@ export type ParagraphAnchorsContext = { columnWidth: number; pageWidth: number; pageMargins: PageMargins; - columns: { width: number; gap: number; count: number }; + // Carries the resolved column layout through to resolveAnchoredGraphicX, direction included: a + // column-relative anchor in an RTL section resolves against the mirrored geometry. + columns: ColumnLayoutForAnchor; placedAnchoredIds: Set; }; @@ -875,7 +878,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para ) : columnX(state); - const fragment = createAnchoredTableFragment(entry.block, entry.measure, anchorX, anchorY); + const fragment = createAnchoredTableFragment(entry.block, entry.measure, anchorX, anchorY, state.columnIndex); state.page.fragments.push(fragment); registeredAnchoredTablePlacements.set(entry.block.id, { page: state.page, diff --git a/packages/layout-engine/layout-engine/src/layout-table.test.ts b/packages/layout-engine/layout-engine/src/layout-table.test.ts index d2600c57f8..617177eb1e 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.test.ts @@ -4,7 +4,7 @@ import type { BlockId, TableAttrs, TableBlock, TableFragment, TableMeasure } from '@superdoc/contracts'; import { describe, expect, it } from 'bun:test'; -import { layoutTableBlock } from './layout-table.js'; +import { createAnchoredTableFragment, layoutTableBlock } from './layout-table.js'; /** * Creates a dummy table fragment for test scenarios where prior page content is needed. @@ -4954,3 +4954,45 @@ describe('layoutTableBlock', () => { }); }); }); + +describe('createAnchoredTableFragment', () => { + const block = createMockTableBlock(1); + const measure = createMockTableMeasure([500], [40]); + + it('records the flow column it was given', () => { + // An anchored table's `x` does not identify its column. `resolveAnchoredGraphicX` places one + // justified to `end` at `col.x + (col.width - width)`, a NEGATIVE offset from its own column + // once the table is wider than it — and `end` is the default for any `w:bidiVisual` table — so + // an over-wide table BEGINS inside an earlier column without ever having left its own. + // Measured on 2 equal columns over a 624px content area (288px each, 48px gutter): a 500px + // table owned by column 1 lands at content x 124, inside column 0. Three consumers had to guess + // the column from that origin and all three guessed wrong — balancing's fragment ordering, + // `determineTableColumn` for hit testing, and the painter's column-separator gate. + const fragment = createAnchoredTableFragment(block, measure, 220, 300, 1); + + expect(fragment.columnIndex).toBe(1); + expect(fragment.isAnchored).toBe(true); + expect(fragment.x).toBe(220); + }); + + it('omits the record entirely when the caller has no column context', () => { + // Absent is NOT column 0. The separator gate reads this field ahead of any geometry, so a + // fabricated 0 would be evidence the fragment never carried — and on a page whose text never + // left the first column that is exactly the difference between a rule and no rule. + const fragment = createAnchoredTableFragment(block, measure, 220, 300); + + expect('columnIndex' in fragment).toBe(false); + expect(fragment.columnIndex).toBeUndefined(); + }); + + it('rejects a column that is not a usable ordinal', () => { + // Same reasoning as above, one step further out: NaN and Infinity reach here from arithmetic on + // a malformed anchor, and writing either would make the field unusable rather than absent. + expect(createAnchoredTableFragment(block, measure, 0, 0, Number.NaN).columnIndex).toBeUndefined(); + expect(createAnchoredTableFragment(block, measure, 0, 0, Number.POSITIVE_INFINITY).columnIndex).toBeUndefined(); + // A fractional or negative ordinal is floored into range rather than dropped, because it still + // names a column the caller meant. + expect(createAnchoredTableFragment(block, measure, 0, 0, 1.9).columnIndex).toBe(1); + expect(createAnchoredTableFragment(block, measure, 0, 0, -3).columnIndex).toBe(0); + }); +}); diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 9994b5d013..8d5dec9de2 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -1946,12 +1946,23 @@ export function layoutTableBlock(context: TableLayoutContext): void { /** * Create a table fragment for an anchored/floating table at its computed position. * Called by the layout engine after the float manager computes the table's position. + * + * `columnIndex` is the flow column the table belongs to, which for an anchored table is NOT + * recoverable from `x`. `resolveAnchoredGraphicX` places one that justifies to `end` at + * `col.x + (col.width - width)`, a negative offset from its own column whenever the table is wider + * than it — and `end` is the default for any `w:bidiVisual` table — so an over-wide table BEGINS + * inside an earlier column without ever having left its own. Every consumer that has to infer the + * column from coordinates gets that wrong: it reorders a balanced page, it answers the wrong column + * for a click, and it decides whether a column separator is drawn. The in-flow table paths have + * always recorded the field for exactly this reason; the anchored ones did not, which is why the + * inference existed at all. */ export function createAnchoredTableFragment( block: TableBlock, measure: TableMeasure, x: number, y: number, + columnIndex?: number, ): TableFragment { const metadata = generateFragmentMetadata(measure, block, 0, block.rows.length, 0); @@ -1967,6 +1978,11 @@ export function createAnchoredTableFragment( height: measure.totalHeight ?? 0, metadata, sourceAnchor: block.sourceAnchor, + // Optional so a caller with no column context omits it rather than asserting column 0, which + // the separator gate would read as content the page does not have. + ...(typeof columnIndex === 'number' && Number.isFinite(columnIndex) + ? { columnIndex: Math.max(0, Math.floor(columnIndex)) } + : {}), }; applyTableFragmentPmRange(fragment, block, measure); return fragment; diff --git a/packages/layout-engine/layout-engine/src/section-breaks.test.ts b/packages/layout-engine/layout-engine/src/section-breaks.test.ts index 303cd16706..e2e7c9a08e 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.test.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.test.ts @@ -141,7 +141,7 @@ describe('scheduleSectionBreak', () => { expect(result.state.pendingColumns).toEqual({ count: 2, gap: 48 }); }); - it('does not trigger mid-page region change for explicit gaps-only changes before geometry uses gaps', () => { + it('triggers a mid-page region change for an explicit gaps-only delta', () => { const state = createSectionState({ activeColumns: { count: 3, gap: 48, widths: [100, 100, 300], gaps: [48, 48], equalWidth: false }, }); @@ -152,7 +152,10 @@ describe('scheduleSectionBreak', () => { const result = scheduleSectionBreak(block, state, BASE_MARGINS); - expect(result.decision.forceMidPageRegion).toBe(false); + // Per-column gaps drive geometry, so a gaps-only delta moves every column after the first + // and needs its own region. It used to compare equal, and the new section was then laid out + // with the previous one's gutters. + expect(result.decision.forceMidPageRegion).toBe(true); expect(result.state.pendingColumns).toEqual({ count: 3, gap: 48, diff --git a/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts index 564b0f863e..1572c5e215 100644 --- a/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it } from 'vite-plus/test'; import { createTestPainter as createDomPainter } from './_test-utils.js'; -import type { ColumnRegion, Fragment, Layout, Page } from '@superdoc/contracts'; +import type { ColumnRegion, Fragment, FlowBlock, Layout, Measure, Page } from '@superdoc/contracts'; // These tests pin down DomPainter's column-separator rendering: // - the fallback path (page.columns only, no mid-page regions) @@ -51,6 +51,44 @@ const paintOnce = (layout: Layout, mount: HTMLElement): void => { painter.paint(layout, mount); }; +// Like paintOnce, but registers extra blocks/measures first. `_test-utils`'s +// paint() auto-synthesizes a block+measure for any 'para' fragment (see +// createTestPainter in _test-utils.ts), but NOT for image/drawing/table +// fragments — resolveImageItem (layout-resolved/resolveImage.ts) throws +// "Missing block/measure entry" without a matching entry, so anchored-float +// fixtures below must supply one explicitly. +const paintWithBlocks = (layout: Layout, mount: HTMLElement, blocks: FlowBlock[], measures: Measure[]): void => { + const painter = createDomPainter({ blocks, measures }); + painter.paint(layout, mount); +}; + +// Minimal image block/measure pair for anchored-float fixtures. `resolveImageItem` +// only checks the block/measure KIND matches ('image'/'image'); it never reads +// width/height off either — those come straight from the fragment — so one +// fixed pair covers every float test below regardless of the fragment's own size. +const FLOAT_BLOCK_ID = 'float-fixture'; +const floatBlock: FlowBlock = { + kind: 'image', + id: FLOAT_BLOCK_ID, + src: 'data:image/gif;base64,R0lGODlhAQABAAAAACw=', + attrs: {}, +}; +const floatMeasure: Measure = { kind: 'image', width: 10, height: 10, scale: 1, naturalWidth: 10, naturalHeight: 10 }; + +// An anchored (floating) image fragment at a given page x/width. `isAnchored: true` +// is what `renderColumnSeparators`'s gate loop reads directly off `item.fragment` +// (FIX 1); `columnIndex` is never set here, so — pre-fix — attribution falls +// through to `columnOwningSpan` exactly like an ordinary fragment would. +const floatAt = (x: number, width: number, y: number = 100): Fragment => ({ + kind: 'image', + blockId: FLOAT_BLOCK_ID, + x, + y, + width, + height: 10, + isAnchored: true, +}); + describe('DomPainter renderColumnSeparators', () => { let mount: HTMLElement; @@ -83,6 +121,199 @@ describe('DomPainter renderColumnSeparators', () => { expect(seps[0].style.height).toBe('864px'); }); + it('gates an RTL separator on the LEFT column, which is the later one there', () => { + // In an RTL section column 0 sits on the right, so "content past the separator" — the + // condition Word uses to decide whether to draw the line at all — is content to its LEFT. + // With the LTR test, a fragment that never left the FIRST column satisfies `x >= separatorX` + // and the painter draws a line Word does not draw. + const firstColumnOnly = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [fragAt(432)], + }); + paintOnce(buildLayout(firstColumnOnly), mount); + expect(querySeparators(mount)).toHaveLength(0); + + mount.remove(); + mount = document.createElement('div'); + document.body.append(mount); + + const bothColumns = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [fragAt(432), fragAt(96)], + }); + paintOnce(buildLayout(bothColumns), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + // Equal columns fill the content area, so the gutter — and the line in it — is where it was. + expect(seps[0].style.left).toBe('408px'); + }); + + it('does not let a page-wide anchored graphic satisfy the RTL gate', () => { + // `page.items` carries anchored drawings as well as column content, and a page-relative + // watermark sits at x = 0 spanning the whole page. Testing its LEFT edge against the + // separator makes it 'past' the separator in RTL while the same item is never past it in + // LTR, so a section whose text never left the first column would draw a line Word does not. + const watermark: Fragment = { ...fragAt(0), width: 816 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [fragAt(432), watermark], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('draws the RTL separator for a wide right-aligned table whose x is OUTSIDE its column', () => { + // The real shape of over-wide content, not the idealised one. `resolveTableFrame` right-aligns + // a table inside its column, and `end` is the default justification for any bidiVisual table, + // so an RTL table wider than its column gets a NEGATIVE offset: it starts left of its own + // column and ends past the separator. Neither of its edges identifies the column it belongs + // to, and neither does its origin. `columnIndex` — which the engine records as it lays the + // fragment out — does. + const wideRtlTable: Fragment = { ...fragAt(-116), width: 500, columnIndex: 1 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [{ ...fragAt(432), columnIndex: 0 }, wideRtlTable], + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.left).toBe('408px'); + }); + + it('still ignores an over-wide table that belongs to the FIRST column', () => { + // The other half of the contract: overflowing out of column 0 is not evidence that a later + // column holds anything, whichever direction the columns run and wherever the box lands. + const rtl = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [{ ...fragAt(220), width: 500, columnIndex: 0 }], + }); + paintOnce(buildLayout(rtl), mount); + expect(querySeparators(mount)).toHaveLength(0); + + mount.remove(); + mount = document.createElement('div'); + document.body.append(mount); + + const ltr = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [{ ...fragAt(96), width: 500, columnIndex: 0 }], + }); + paintOnce(buildLayout(ltr), mount); + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('counts a fragment nudged out of its column by a negative indent', () => { + // A negative `w:ind` puts a paragraph's origin in the gutter, outside every column span. The + // engine still knows which column it belongs to, so the line must be drawn. + const outdented: Fragment = { ...fragAt(422), columnIndex: 1 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [{ ...fragAt(96), columnIndex: 0 }, outdented], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(1); + }); + + it('attributes a fragment on a zero-gap column boundary to the LATER column', () => { + // With `w:space="0"` adjacent columns share an endpoint, and that endpoint is exactly where + // the later column's content starts. Column spans are half-open so the boundary belongs to + // the column that begins there, not the one that ends there. + const page = buildPage({ + columns: { count: 2, gap: 0, withSeparator: true }, + fragments: [fragAt(96), fragAt(408)], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(1); + }); + + it('does not let a page-wide anchored graphic satisfy the LTR gate either', () => { + // The watermark guard is not an RTL special case: an item that belongs to no column is not + // evidence for any separator, whichever way the columns run. + const watermark: Fragment = { ...fragAt(0), width: 816 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), watermark], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('still draws the separator when the later column holds only an outdented paragraph', () => { + // The paginator records `columnIndex` for tables and footnote bodies but not for ordinary + // paragraphs, so a paragraph reaches the geometry fallback. A negative `w:ind` puts its + // origin in the gutter, outside its own column: attributing by containment of the origin + // would find no column and suppress a line Word draws. + // 2 equal columns of 288 in a 624 content area: column 1 starts at 96 + 288 + 48 = 432. + const outdented: Fragment = { ...fragAt(432 - 40), width: 288 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), outdented], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(1); + }); + + it('ignores content that overflows the FIRST column when nothing recorded its column', () => { + // The sibling test above pins the same contract for a fragment the engine tagged with + // `columnIndex: 0`. Ordinary paragraphs never carry that tag, so this one goes through the + // geometry fallback — and attributing by overlap alone answers column 1 here: with + // `widths: [100, 400]` a 500px box starting at column 0's own edge covers 100px of column 0 + // and 352px of column 1. Overflowing out of column 0 is not evidence that column 1 holds + // anything, so the origin has to be consulted before the overlap. + const overflowing: Fragment = { ...fragAt(96), width: 500 }; + const page = buildPage({ + columns: { count: 2, gap: 48, widths: [100, 400], equalWidth: false, withSeparator: true }, + fragments: [overflowing], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('does not let a page-wide graphic satisfy the gate when explicit widths overfill the page', () => { + // Explicit widths are floored to >= 1px but never CAPPED, so [150, 600] with a 48px gap + // occupies 798px inside a 624px content area. Measured against the strip's OWN span a + // page-wide item is merely partial, and overlap attribution then hands it to the column it + // covers most: column 1, the wider one (150px of column 0 against 426px of column 1). That is + // exactly the "a later column holds content" the gate asks about, so the line would be drawn + // on a page where Word draws none. Bounding by the page content area as well is what stops it. + // + // Direction-independent: the LTR strip runs 0..150 / 198..798 and the mirrored RTL one + // 474..624 / -174..426, and the graphic wins column 1 in both. + const overfull = { count: 2, gap: 48, widths: [150, 600], equalWidth: false, withSeparator: true }; + // Spans the content area exactly: x = leftMargin, width = 816 - 96 - 96. + const pageWide: Fragment = { ...fragAt(96), width: 624 }; + // Column 0 runs 96..246 in LTR page coordinates and 570..720 in RTL; column 1 runs 294..894 + // and -78..522. The second row is a positive control: ordinary content in the later column + // still draws the line, so the suppression above is not vacuous. + const cases = { + ltr: { first: fragAt(96), later: fragAt(300) }, + rtl: { first: fragAt(570), later: fragAt(200) }, + } as const; + + for (const direction of ['ltr', 'rtl'] as const) { + const { first, later } = cases[direction]; + for (const [fragments, expected] of [ + [[first, pageWide], 0], + [[first, later], 1], + ] as const) { + mount.remove(); + mount = document.createElement('div'); + document.body.append(mount); + + paintOnce(buildLayout(buildPage({ columns: { ...overfull, direction }, fragments })), mount); + expect(querySeparators(mount)).toHaveLength(expected); + } + } + }); + it('draws count-1 separators for 3 equal columns', () => { const page = buildPage({ columns: { count: 3, gap: 48, withSeparator: true }, @@ -366,4 +597,255 @@ describe('DomPainter renderColumnSeparators', () => { expect(seps[0].style.height).toBe('300px'); }); }); + + // Reference geometry for every test below, derived (not assumed) from the real + // normalizeColumnLayout/getColumnGeometry: page 816x1056, margins 96 all round → + // contentWidth = 816 - 96 - 96 = 624. `{count:2, gap:48, withSeparator:true}` in + // equal mode gives availableWidth = 624 - 48 = 576, so each column is 576/2 = 288. + // Column 0 is content-relative [0,288), column 1 is [336,624) (288 + the 48 gap), + // and the separator sits at the gutter midpoint, content x 312. Page x = content x + // + leftMargin(96): col0 → page [96,384), col1 → page [432,720), separator → page + // 408. In an RTL section the same equal-width strip mirrors about the content area + // and lands on the identical page x's — verified in the existing "gates an RTL + // separator on the LEFT column" test above — because column 0's mirrored span + // [336,624) is column 1's un-mirrored span and vice versa. + + describe('FIX 1 - a float never lights the content-presence gate', () => { + // `page.items` is `page.fragments.map(...)` with no anchor filtering (renderer.ts, + // around the `occupiedColumns` loop), and an anchored fragment carries its own + // width — so a narrow float is the ordinary case the gate has to reject, not an + // exception. Verified against the pre-fix gate loop (git HEAD, commit 2438bd9, + // before `if (source?.isAnchored === true) continue;` existed) via a scratch + // replica built on the real normalizeColumnLayout/getColumnGeometry: for every + // case below, the pre-fix loop (no anchor check, `columnOwningSpan` = pure overlap) + // puts the float in column 1, drawing a separator at page x 408; the current gate + // excludes it and draws none. + it('excludes an anchored float whose origin sits inside the other column', () => { + // Body text never leaves column 0 (page x 96, content x 0). A 200px-wide + // anchored float at page x 500 has content x 500-96=404, inside column 1's + // [336,624) span (404 < 624). Pre-fix: columnOwningSpan(404, 200) — overlap + // with col0 is min(604,288)-max(404,0) = -116 → 0; overlap with col1 is + // min(604,624)-max(404,336) = 200. Column 1 wins on overlap alone, occupied + // becomes {0,1}, and column 0's separator (content x 312) draws because a + // LATER column (1) is occupied. Post-fix the float is skipped before + // `columnOwningSpan` ever runs, occupied stays {0}, and the gate stays shut. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), floatAt(500, 200)], + }); + paintWithBlocks(buildLayout(page), mount, [floatBlock], [floatMeasure]); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('excludes an anchored float sitting entirely in the gutter', () => { + // A 40px float at page x 416 has content x 416-96=320, inside the gutter + // (288 <= 320 < 336) — outside BOTH columns' own spans. Pre-fix overlap still + // awards it column 1: overlap with col0 is min(360,288)-max(320,0) = -32 → 0; + // overlap with col1 is min(360,624)-max(320,336) = 24 > 0, so column 1 wins by + // the only nonzero margin. Same outcome as the wider float above — occupied + // {0,1} pre-fix draws the separator, {0} post-fix does not — confirming the + // exclusion isn't just catching floats that already sit in a column's span. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), floatAt(416, 40)], + }); + paintWithBlocks(buildLayout(page), mount, [floatBlock], [floatMeasure]); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('excludes an anchored float in an RTL section too', () => { + // Mirrored geometry: column 0 (fill-order first) is on the RIGHT, content + // [336,624) / page [432,720); column 1 is on the LEFT, content [0,288) / page + // [96,384) (see the reference-geometry note above this describe block, and the + // existing "gates an RTL separator on the LEFT column" test, which pins the + // same mirrored spans). Body text stays in column 0 (page x 432). A 200px + // float at page x 150 has content x 150-96=54, entirely inside column 1's + // [0,288) span (54+200=254 < 288) — overlap picks it with no ambiguity: overlap + // with col1 is the full 200, overlap with col0 is 0. Pre-fix that occupies + // column 1 and draws the separator (content x 312 either direction, since + // equal columns fill the content area exactly); post-fix the float is skipped + // and only column 0 is occupied, so the gate stays shut — the exclusion is + // direction-independent, matching FIX 1's own reasoning (`hRelativeFrom` never + // reaches the fragment, so every float is excluded, not only page-relative ones). + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true, direction: 'rtl' }, + fragments: [fragAt(432), floatAt(150, 200)], + }); + paintWithBlocks(buildLayout(page), mount, [floatBlock], [floatMeasure]); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('still draws the separator for a NON-anchored fragment at the same position (guard)', () => { + // Positive control for the exclusion above, same page x 500 / width 200 as the + // first case, but the fragment is ordinary column content (isAnchored omitted). + // This is not expected to fail against pre-fix HEAD — it doesn't: both the + // pre-fix pure-overlap columnOwningSpan and the current one attribute this box + // to column 1 (overlap/fit both favor it, since the box sits entirely past + // column 0), so both draw the separator. It's here as a guard against + // over-breadth: proving the FIX 1 exclusion is keyed on `isAnchored` + // specifically, not on "any narrow box past the first column." + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), { ...fragAt(500), width: 200 }], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(1); + expect(querySeparators(mount)[0].style.left).toBe('408px'); + }); + }); + + describe('FIX 2 - an out-of-range recorded columnIndex is rejected, not clamped', () => { + it('rejects columnIndex:5 on a 2-column page rather than clamping it to column 1', () => { + // The only fragment on the page sits at page x 96 (content x 0), which geometry + // alone attributes to column 0. It also carries a stale/corrupt `columnIndex: 5` + // — there is no column 5 on a 2-column page (lastColumnIndex is 1). Pre-fix (git + // HEAD): `Math.max(0, Math.min(1, Math.floor(5)))` clamps that to column 1, + // occupying it and drawing the separator at page x 408 even though nothing is + // really there. Post-fix: 5 is outside [0, lastColumnIndex] after flooring, so + // the record is rejected outright and attribution falls through to geometry, + // which (correctly) says column 0 — leaving column 1 unoccupied and the gate shut. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [{ ...fragAt(96), columnIndex: 5 }], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('still lets a valid recorded columnIndex beat geometry (guard)', () => { + // Same page x 96 (content x 0) that geometry alone would call column 0, but + // this time columnIndex:1 is IN range (0 <= 1 <= lastColumnIndex 1). This is not + // expected to fail against pre-fix HEAD — it doesn't: a valid record was never + // clamped either version, only an out-of-range one, so both accept it and draw + // the separator. It's here to guard the boundary the fix drew: rejection is for + // out-of-range records specifically, not for every mismatch between the record + // and where geometry would have placed the fragment. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [{ ...fragAt(96), columnIndex: 1 }], + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.left).toBe('408px'); + }); + + it('floors a near-integer columnIndex before range-checking it (guard)', () => { + // columnIndex: 1.0000001 (ordinary float drift, not corruption) floors to 1, + // which IS in range, and resolves to column 1 — it must not be discarded as + // "out of range" by comparing the unfloored 1.0000001 against lastColumnIndex + // first. Not expected to fail against pre-fix HEAD — it doesn't: the pre-fix + // clamp expression also floors before comparing (`Math.floor(owned)` is the + // innermost call in both versions), so this pins the flooring order rather than + // distinguishing the two. Kept as a guard because the reject-vs-clamp rewrite + // touched this exact expression and a reordering slip here would be silent. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [{ ...fragAt(96), columnIndex: 1.0000001 }], + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.left).toBe('408px'); + }); + }); + + describe('a box wider than its column is the only origin the gate distrusts', () => { + // Why WIDTH and not the right edge, stated once for both cases below. An edge gate would be + // dead code: pass it and the box lies wholly inside one column's span, and since + // `getColumnGeometry` never emits overlapping spans, every other column's overlap is zero and + // the vote below returns the same column anyway. Swept over outdents from 0 to 160px in 2px + // steps, an edge-gated containment step and plain overlap never disagreed once. The width gate + // is what makes the step do work, because it admits the one shape whose right edge overhangs + // while its origin is still authoritative -- the right-aligned frame in the first test. + it('keeps a right-aligned framed paragraph in the column its origin is in', () => { + // `w:framePr` with `xAlign="right"` re-points the fragment at + // `columnX + (effectiveColumnWidth - maxLineWidth)` (layout-paragraph.ts, `floatAlignment`) + // and leaves `width` at the FULL column width, so the recorded box overhangs the gutter and + // the next column while the text never left column 0. Two equal 288px columns over 624 put a + // frame whose longest line is 50px at content-relative 238 with width 288, i.e. the box + // [238, 526]: neither edge lands on a column edge, and overlap alone favours the neighbour -- + // 50px of column 0 against 190px of column 1 -- so the gate drew a separator on a page with + // nothing in its second column. + // + // This is why origin containment is gated on the box's WIDTH and not on its right edge. The + // box is 288 wide against a 288px column, so it fits, and its origin is believed. Gating on + // the right edge rejects it (526 > 288) and hands it to the overlap vote, which is wrong. + const framed: Fragment = { ...fragAt(96 + 288 - 50), width: 288 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), framed], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('still distrusts the origin of a box that outgrew its column', () => { + // The shape that actually reaches the gate's rejection path. `resolveTableFrame` centres an + // over-wide table inside its column, at `col.x + (col.width - width) / 2` -- a NEGATIVE offset + // once the table is wider than the column -- so it begins inside an earlier column without + // ever having left its own. A 400px box centred in column 1 of two equal 288px columns over + // 624 is [280, 680]: neither edge lands on a column edge (680 misses column 1's 624 by 56), + // the origin 280 falls inside column 0, and 400 does not fit a 288px column. So the origin is + // rejected and the overlap vote answers column 1, 288px against 8px. + // + // An outdented paragraph does NOT reach here, though this test used one until cubic pointed + // out that it could not. A negative `w:ind` widens the fragment by exactly the outdent it + // shifts by, so `x + width` lands on its own column's trailing edge for every outdent and the + // trailing-edge rule answers first. Worth recording rather than quietly swapping the fixture: + // that outdent was the only guard on this gate, and replacing the width comparison with + // unconditional origin trust left all 39 tests in this file passing. + const centredOverWide: Fragment = { ...fragAt(96 + 280), width: 400 }; + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: true }, + fragments: [fragAt(96), centredOverWide], + }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(1); + }); + }); + + describe('FIX 4 - columnOwningSpan folds the geometry bound instead of spreading it', () => { + it('does not throw for a six-figure column count', () => { + // `Math.min(...arr)`/`Math.max(...arr)` pass every element as a call argument, + // and V8 has a hard argument-count ceiling on that: a scratch check on this + // machine (plain `node -e`, this repo's pinned Node) found the largest array + // Math.min(...arr) still accepts is 124729 elements — 124730 throws + // `RangeError: Maximum call stack size exceeded`. 150000 is comfortably past + // that measured threshold (and the task-suggested count), with margin for a + // deeper call stack inside the real test runner. + // + // Reaching columnOwningSpan at all takes a layout that survives + // resolveSeparatorColumnGeometry's OWN pre-geometry guard: equal-mode columns + // are rejected pre-geometry when (contentWidth - gap*(count-1))/count <= 1. + // With gap 0 that requires contentWidth > count, so this page is 200020px wide + // with 10px margins (contentWidth 200000) against a 150000-column layout — + // equalWidth = 200000/150000 ≈ 1.33, just over the guard, and each of the + // 150000 columns floors to that same ~1.33px width, so geometry.some(w<=1) + // (the other pre-existing guard) doesn't reject it either. A single fragment at + // the content origin (page x 10) is enough to reach columnOwningSpan — it isn't + // testing WHICH column wins, only that resolving one doesn't crash the paint. + // + // Confirmed via the scratch replica against this exact 150000-column geometry: + // the pre-fix (git HEAD) columnOwningSpan throws `RangeError: Maximum call + // stack size exceeded` on `Math.min(...geometry.map(...))`; the current, + // fold-based one returns a plain column index with no throw. + const page = buildPage({ + margins: { top: 10, right: 10, bottom: 10, left: 10 }, + columns: { count: 150000, gap: 0, withSeparator: true }, + fragments: [{ ...fragAt(10), width: 100 }], + }); + + expect(() => paintOnce(buildLayout(page, { w: 200020, h: 400 }), mount)).not.toThrow(); + }); + }); }); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index ce9fa00941..06dcc4e977 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1,6 +1,7 @@ import type { ChartDrawing, CellBorders, + ColumnGeometry, ColumnLayout, CustomGeometryData, DrawingBlock, @@ -54,8 +55,8 @@ import { expandRunsForInlineNewlines, formatPageNumber, formatSectionPageNumberText, + findColumnContaining, getColumnGeometry, - getColumnSeparatorPositions as getColumnSeparatorPositionsFromGeometry, isPositionedParagraphFrame, normalizeColumnLayout, resolveColumnMode, @@ -1053,6 +1054,146 @@ function svgEffectColor(value: TextEffectColor): string | undefined { * - Incremental re-rendering when only specific blocks change * - Hyperlink rendering with security sanitization and accessibility */ +/** + * The column that owns a fragment spanning `[x, x + width)` in content-relative coordinates, or + * `null` when it belongs to no column. + * + * Four rules after the width bound, in this order, because each is wrong for the case the next one + * answers. `balanceSectionOnPage`'s `ordinalOf` asks the same four for the same reasons; the two + * differ only at the end, where a sort key must name a column and this may answer `null`. + * + * 0. WIDTH BOUND. Anything at least as wide as the area it would have to be content OF belongs to no + * column. That is what keeps page-anchored objects out of the gate: `page.items` carries them, + * and a full-width watermark overlaps every column without being content of any. It comes first, + * before every rule below: in a mirrored RTL strip the LAST column reaches furthest left, so a + * watermark at x = 0 sits on that column's leading edge and would be read as content of it. + * + * The area has two bounds and the threshold is the smaller, because either alone leaks. The + * strip's own span, since explicit widths are not scaled to fill the page (see + * `normalizeColumnLayout`) and an underfilling strip is narrower than the content area. And the + * page content area, since those widths are not CAPPED either — nothing clamps their sum — so an + * authored `w:num="2"` with two over-wide `w:col/@w` produces a strip WIDER than the page, and + * against the strip bound alone a page-wide graphic measures as merely partial. + * + * 1. LEADING EDGE. A box that starts on a column's own edge is that column's, fit or no fit. That is + * ordinary content, and it is also content WIDER than its column, which overflows from that same + * edge. Asked before any overlap test, because an over-wide box can cover more of a wide + * neighbour than of the narrow column it came from: `widths: [100, 400]` puts a 500px box that + * starts at column 0's edge 100px into column 0 and 352px into column 1. + * + * 2. TRAILING EDGE — a different question, not a mirror of the first. An indent moves only the + * leading edge, so a paragraph outdented FURTHER than the gutter has its origin inside the + * PREVIOUS column while still ending exactly at its own column's trailing edge. And + * `resolveTableFrame` places an over-wide table justified to `end` at a negative offset from its + * own column, which likewise begins in an earlier column without ever having left its own. + * + * 3. THE ORIGIN, while the box still FITS the column it starts in. The fit is what makes the origin + * evidence: a smaller indent leaves the origin inside its own column and the box inside it too. A + * centred over-wide table also begins inside an earlier column, and there the origin is no + * evidence at all — which is what the fit test rejects. + * + * 4. OVERLAP, for a box whose origin sits in no column: hung into a gutter by a negative `w:ind` or + * a float offset. Ties go to the earliest column in fill order, which arises only for an overfull + * strip whose columns genuinely overlap. + * + * `null` at the end is the safe answer here, unlike in `ordinalOf`, which must clamp because a sort + * key that is sometimes absent is not a total order. The question here is "does a LATER column hold + * content", so `null` can only ever suppress a separator, never invent one. + */ +/** + * Sub-pixel slack for "this edge IS that column's edge". Column x values reach fragments through + * `getColumnX`, so unindented content matches exactly and this only absorbs float drift — it stays + * two orders of magnitude below the smallest indent a document can author (1 twip = 1/1440in). + */ +const COLUMN_EDGE_EPSILON = 0.01; + +function columnOwningSpan(geometry: ColumnGeometry[], x: number, width: number, contentWidth: number): number | null { + if (geometry.length === 0) return null; + + const span = Number.isFinite(width) && width > 0 ? width : 0; + // Folded rather than spread into `Math.min`/`Math.max`: `w:num` is bounded at 45 by the schema but + // nothing in the pipeline enforces it, and a host-built layout with a six-figure count overflows + // the argument stack — a paint-time crash, from a function whose whole job is to answer + // conservatively. + let stripStart = Infinity; + let stripEnd = -Infinity; + for (const col of geometry) { + if (col.x < stripStart) stripStart = col.x; + if (col.x + col.width > stripEnd) stripEnd = col.x + col.width; + } + // The page bound applies only when the page reports a usable content width. A malformed page can + // report zero or a negative one, and letting that become the threshold would reject every fragment + // and blank out separators that belong on the page. + const pageSpan = Number.isFinite(contentWidth) && contentWidth > 0 ? contentWidth : Infinity; + if (span >= Math.min(stripEnd - stripStart, pageSpan)) return null; + + // A box on a column's LEADING edge is that column's, fit or no fit. That covers ordinary content, + // and it covers content WIDER than its column, which overflows rightward from that same edge in + // either direction. Asked before any overlap test, because an over-wide box can cover more of a + // wide neighbour than of the narrow column it came from: `widths: [100, 400]` puts a 500px box + // that starts at column 0's edge 100px into column 0 and 352px into column 1. + for (const col of geometry) { + if (Math.abs(x - col.x) <= COLUMN_EDGE_EPSILON) return col.index; + } + + // A box on a column's TRAILING edge is that column's too, and it is a different question rather + // than a mirror of the one above. An indent moves only the leading edge, so a paragraph outdented + // FURTHER than the gutter has its origin inside the previous column while still ending exactly at + // its own column's trailing edge — measured on equal 2-col geometry over 624px (col0 [0,288), + // col1 [336,624)): a column-1 paragraph outdented 72px is the box [264, 624]. And + // `resolveTableFrame` places an over-wide table justified to `end` at a NEGATIVE offset from its + // own column, which likewise begins inside an earlier column without ever having left its own. + for (const col of geometry) { + if (Math.abs(x + span - (col.x + col.width)) <= COLUMN_EDGE_EPSILON) return col.index; + } + + // Containment of the origin, gated on the box being no WIDER than the column it starts in — not on + // its right edge landing inside that column. The distinction is the whole content of this step. + // + // A box that fits its column was placed in that column wherever the origin ended up: ordinary + // content, a `w:ind` indent, and — the case that makes this step load-bearing — a paragraph + // carrying `attrs.floatAlignment` of `right` or `center`. `layout-paragraph.ts` re-points such a + // fragment at `columnX + (effectiveColumnWidth - maxLineWidth)` and never reduces + // `fragment.width`, so a 50px line in a 288px column is recorded as x = columnX + 238 with width + // still 288: origin inside its own column, right edge 238px past it. An edge gate rejects that, + // and the overlap vote below then sees 50px of column 0 against 190px of column 1 and moves it — + // so a page whose text never left column 0 drew a separator, with no anchored object involved at + // all. (Nothing in this repo's production code sets `floatAlignment`; it arrives from the + // adapter outside the layout engine, so the OOXML feature behind it is deliberately not named.) + // + // A box WIDER than its column may instead have been pulled OUT of it, and there the origin is no + // evidence at all. `resolveTableFrame` centres an over-wide table inside its column, placing it at + // `col.x + (col.width - width) / 2` — a NEGATIVE offset once the table is wider than the column — + // so it begins inside an EARLIER column without ever having left its own. Measured on equal 2-col + // geometry over 624px (col0 [0,288), col1 [336,624)): a 400px box centred in column 1 is + // [280, 680], whose origin is in column 0 and whose overlap correctly answers 1, 288px against + // 8px. Width is what separates the two shapes; the right edge overhangs in both. + // + // An outdented paragraph is NOT this case, though it looks like it and was written here as the + // justification once. A negative `w:ind` widens the fragment by exactly the outdent it shifts by, + // so `x + width` lands on its own column's trailing edge for EVERY outdent — the rule above has + // already answered and this step never sees it. That mistake is worth recording rather than just + // deleting: it was also the fixture guarding this gate, so the gate had no test at all. Replacing + // the width comparison with unconditional origin trust left all 39 tests in + // `renderer-column-separators.test.ts` passing. + const byOrigin = findColumnContaining(geometry, x); + if (byOrigin !== null) { + const originColumn = geometry.find((col) => col.index === byOrigin); + if (originColumn && span <= originColumn.width + COLUMN_EDGE_EPSILON) return byOrigin; + } + + let best: number | null = null; + let bestOverlap = 0; + for (const col of geometry) { + const overlap = Math.min(x + span, col.x + col.width) - Math.max(x, col.x); + if (overlap > bestOverlap) { + bestOverlap = overlap; + best = col.index; + } + } + return best; +} + export class DomPainter { private readonly options: PainterOptions; private mount: HTMLElement | null = null; @@ -1782,20 +1923,79 @@ export class DomPainter { const regionHeight = yEnd - yStart; if (regionHeight <= 0) continue; - const separatorPositions = this.getColumnSeparatorPositions(columns, leftMargin, contentWidth); - if (separatorPositions.length === 0) continue; + const geometry = this.resolveSeparatorColumnGeometry(columns, contentWidth); + if (!geometry) continue; // Word only renders the column separator between columns that both have // content. For a 2-col page where col 1 is empty (e.g. the last page of // a multi-column section that fits in col 0, or a `nextPage` section // where Word fills col 0 first without balancing), Word draws no line // even when the section's `w:cols` declared `w:sep="1"`. Gate each - // separator on whether any fragment sits past it within the region. + // separator on whether any LATER column in fill order holds content. const fragmentsInRegion = page.items.filter((item) => item.y >= yStart - 0.5 && item.y < yEnd + 0.5); - for (const separatorX of separatorPositions) { - const hasContentPastSeparator = fragmentsInRegion.some((f) => f.x >= separatorX); + // Ask which column OWNS each fragment rather than comparing an edge against the separator. + // An edge test has to pick which edge trails in the fill direction, and no choice is right: + // content wider than its column does not sit inside it. `resolveTableFrame` places a + // right-aligned or centred over-wide table at a NEGATIVE offset from its column — and `end` + // is the default justification for any bidiVisual table — so in an RTL section such a table + // starts left of its own column and ends past the separator, while never having left the + // later column at all. + // + // `fragment.columnIndex` is the engine's own record of the owning column, and it is the + // first thing consulted. It reaches only a few fragment kinds: tables (`layout-table.ts`, + // five sites), the three footnote body kinds in `incrementalLayout.ts`, and a paragraph ONLY + // when it is a collapsed split-line-break anchor carrier (`layout-paragraph.ts`, under + // `collapseSplitLineBreakCarrier`) — a narrow document shape, not the ordinary paragraph. So + // the geometry fallback below carries almost every fragment on the page and has to be right + // on its own; the record is a shortcut for the cases that keep one, not the main path. + const lastColumnIndex = geometry.length - 1; + const occupiedColumns = new Set(); + for (const item of fragmentsInRegion) { + // `page.items` are paint items; the engine`s record of the owning column lives on the + // source fragment they point back to. + const source = (item as { fragment?: { columnIndex?: number; isAnchored?: boolean } }).fragment; + + // A FLOAT is not column content, and the width threshold below cannot recognise one. That + // threshold catches a full-width watermark, which is the case it was written for, but + // `page.items` is `page.fragments.map(...)` with no anchor filtering, and an anchored object + // carries its own `measure.width` — so a narrow one is the ordinary case, not the exception. + // A 200px logo placed at page x 500 on a 2-column page whose text never leaves column 0 has + // its origin inside column 1 and lit this gate, drawing a rule Word does not draw. The same + // logo 84px further left lands in the gutter and wins column 1 on overlap instead. + // + // Every float is excluded, not only the page-relative ones. `hRelativeFrom` is consumed at + // layout time and never reaches the fragment, so telling a page-anchored float from a + // column-anchored one here means reaching back through `item.block`, and I have no evidence + // about the case that would distinguish them: whether Word draws a rule beside a column + // holding a floating object and no text. Word's rule tracks text, and the gate is + // deliberately asymmetric — excluding an item can only ever suppress a rule, never invent + // one — so the conservative reading is also the simpler one. If a document turns up where + // Word draws that rule, the fix is to admit column-anchored floats specifically. + if (source?.isAnchored === true) continue; + + // An out-of-range record is REJECTED, not clamped. Clamping turned any stale or corrupt value + // into a real column index — `columnIndex: 5` on a two-column page became 1 — which is + // exactly the "a later column holds content" this gate asks about, invented out of a number + // that describes no column on this page. Falling through to geometry answers from the + // fragment's actual position instead. Flooring first, so ordinary float drift on a valid + // index still resolves rather than being thrown away as out of range. + const owned = source?.columnIndex; + const recorded = typeof owned === 'number' && Number.isFinite(owned) ? Math.floor(owned) : null; + const columnIndex = + recorded !== null && recorded >= 0 && recorded <= lastColumnIndex + ? recorded + : columnOwningSpan(geometry, item.x - leftMargin, item.width, contentWidth); + if (columnIndex !== null) occupiedColumns.add(columnIndex); + } + + // Iterating the geometry (rather than a positions array) keeps each separator paired with the + // column it follows, which is what "a later column" is measured against. + for (const column of geometry) { + if (column.separatorX === undefined) continue; + const hasContentPastSeparator = [...occupiedColumns].some((index) => index > column.index); if (!hasContentPastSeparator) continue; + const separatorX = leftMargin + column.separatorX; const separatorEl = this.doc.createElement('div'); separatorEl.dataset.superdocColumnSeparator = 'true'; @@ -1812,7 +2012,12 @@ export class DomPainter { } } - private getColumnSeparatorPositions(columns: ColumnLayout, leftMargin: number, contentWidth: number): number[] { + /** + * The resolved column geometry this region's separators are drawn from, or null when the region + * draws none. Returns the geometry rather than bare x positions because the gate needs to know + * WHICH column each separator follows, not only where it sits. + */ + private resolveSeparatorColumnGeometry(columns: ColumnLayout, contentWidth: number): ColumnGeometry[] | null { // SD-2629: separator positions come from the one resolved column geometry (the same source as // fill count and column widths), not a re-derivation here. The caller has already gated on // withSeparator and count > 1. @@ -1824,12 +2029,12 @@ export class DomPainter { // raw equalWidth:true config carrying stray widths still takes the equal-mode guard. Legacy guard. if (resolveColumnMode(columns) === 'equal') { const equalWidth = (contentWidth - columns.gap * (normalized.count - 1)) / normalized.count; - if (equalWidth <= 1) return []; + if (equalWidth <= 1) return null; } const geometry = getColumnGeometry(normalized); - if (geometry.length <= 1) return []; - if (geometry.some((column) => column.width <= 1)) return []; - return getColumnSeparatorPositionsFromGeometry(geometry, leftMargin); + if (geometry.length <= 1) return null; + if (geometry.some((column) => column.width <= 1)) return null; + return geometry; } private renderDecorationsForPage(pageEl: HTMLElement, page: ResolvedPage, pageIndex: number): void { if (this.isSemanticFlow) return; diff --git a/packages/layout-engine/tests/src/section-breaks-rtl-columns.test.ts b/packages/layout-engine/tests/src/section-breaks-rtl-columns.test.ts new file mode 100644 index 0000000000..26179f7c94 --- /dev/null +++ b/packages/layout-engine/tests/src/section-breaks-rtl-columns.test.ts @@ -0,0 +1,193 @@ +/** + * RTL Section Column Order Tests + * + * A section carrying `w:sectPr/w:bidi` fills its columns right to left: the first paragraph belongs + * in the RIGHT column and overflow spills into the left one (ECMA-376 §17.6.1). Column widths, the + * gutter and the text direction inside each column are governed elsewhere and must not move. + * + * Regression coverage for the issue where fill order was a fixed left-to-right and the section + * direction was never consulted on the column axis. + * + * @module section-breaks-rtl-columns.test + */ + +import { describe, it, expect, beforeEach } from 'vite-plus/test'; +import type { Layout } from '@superdoc/contracts'; +import { + createPMDocWithSections, + convertAndLayout, + pmToFlowBlocks, + getSectionBreaks, + PAGE_SIZES, + resetBlockIdCounter, + type TestSectionProps, +} from './test-helpers/section-test-utils.js'; + +/** Enough numbered paragraphs to overflow the first column, so the fill order is observable. */ +const NUMBERED_PARAGRAPHS = Array.from( + { length: 24 }, + (_, index) => `Paragraph number ${index + 1}. ${'filler '.repeat(20)}`, +); + +const TWO_COLUMN_SECTION: TestSectionProps = { + type: 'nextPage', + pageSize: PAGE_SIZES.LETTER_PORTRAIT, + columns: { count: 2, gap: 48 }, +}; + +const layoutTwoColumnSection = async (props: TestSectionProps): Promise => { + const pmDoc = createPMDocWithSections([{ paragraphs: NUMBERED_PARAGRAPHS }], props); + return convertAndLayout(pmDoc, { pageSize: PAGE_SIZES.LETTER_PORTRAIT }); +}; + +/** `blockId` is `-paragraph`, which is the document order of the source array. */ +const paragraphIndex = (blockId: string): number => Number.parseInt(blockId, 10); + +type ColumnReadout = { + /** Distinct fragment x values on the page, ascending. */ + columnXs: number[]; + /** Paragraph indices in each column, keyed by that column's x, each in visual top-to-bottom order. */ + indicesByX: Map; +}; + +const readFirstPage = (layout: Layout): ColumnReadout => { + const fragments = [...layout.pages[0].fragments] + .filter((fragment) => fragment.blockId.endsWith('-paragraph')) + .sort((a, b) => a.y - b.y); + + const indicesByX = new Map(); + for (const fragment of fragments) { + const x = Math.round(fragment.x); + if (!indicesByX.has(x)) indicesByX.set(x, []); + indicesByX.get(x)!.push(paragraphIndex(fragment.blockId)); + } + + return { columnXs: [...indicesByX.keys()].sort((a, b) => a - b), indicesByX }; +}; + +/** True when `indices` is 0,1,2,… — i.e. this column holds a contiguous prefix of the document. */ +const isAscendingPrefix = (indices: number[]): boolean => indices.every((value, i) => value === i); + +describe('Section Breaks - RTL Column Order', () => { + beforeEach(() => { + resetBlockIdCounter(); + }); + + it('starts an RTL section in the right column and overflows into the left one', async () => { + const layout = await layoutTwoColumnSection({ ...TWO_COLUMN_SECTION, bidi: true }); + const { columnXs, indicesByX } = readFirstPage(layout); + + expect(columnXs).toHaveLength(2); + const [leftX, rightX] = columnXs; + const right = indicesByX.get(rightX)!; + const left = indicesByX.get(leftX)!; + + // Paragraph 1 opens the section, in the RIGHT column. + expect(right[0]).toBe(0); + // The right column holds a contiguous prefix and the left column continues it, so reading + // right-then-left reproduces document order exactly. + expect(isAscendingPrefix(right)).toBe(true); + expect(left).toEqual(left.map((_, i) => right.length + i)); + // Both columns are actually used — otherwise "first column is on the right" proves nothing. + expect(left.length).toBeGreaterThan(0); + }); + + it('leaves an LTR section filling left to right', async () => { + const layout = await layoutTwoColumnSection(TWO_COLUMN_SECTION); + const { columnXs, indicesByX } = readFirstPage(layout); + + const [leftX, rightX] = columnXs; + expect(indicesByX.get(leftX)![0]).toBe(0); + expect(isAscendingPrefix(indicesByX.get(leftX)!)).toBe(true); + expect(indicesByX.get(rightX)![0]).toBeGreaterThan(0); + }); + + it('moves only the order — column widths and the gutter are untouched', async () => { + const ltr = readFirstPage(await layoutTwoColumnSection(TWO_COLUMN_SECTION)); + resetBlockIdCounter(); + const rtl = readFirstPage(await layoutTwoColumnSection({ ...TWO_COLUMN_SECTION, bidi: true })); + + // Identical geometry: the same two column origins, so no width or gutter moved. + expect(rtl.columnXs).toEqual(ltr.columnXs); + + // And an exact mirror of the assignment: whatever LTR put in the left column, RTL puts in the + // right one, paragraph for paragraph. Comparing only the x values or the fragment total would + // pass even with the mirror ripped out, since both are invariant under it. + const [leftX, rightX] = ltr.columnXs; + expect(rtl.indicesByX.get(rightX)).toEqual(ltr.indicesByX.get(leftX)); + expect(rtl.indicesByX.get(leftX)).toEqual(ltr.indicesByX.get(rightX)); + }); + + it('keeps a balanced last page right-to-left', async () => { + // A multi-column section that ends mid-page gets its last page re-balanced, which REBUILDS the + // column geometry and overwrites every fragment's x from it. That rebuild is a separate code + // path from ordinary fill, so it can lose the axis on its own: the tail page of a two-column + // Hebrew section would flip to left-to-right while every earlier page stayed right-to-left. + const balanced = async (bidi: boolean) => { + resetBlockIdCounter(); + const pmDoc = createPMDocWithSections( + [ + { + paragraphs: Array.from({ length: 8 }, (_, i) => `Paragraph number ${i + 1}. ${'word '.repeat(30)}`), + props: { + type: 'continuous', + pageSize: PAGE_SIZES.LETTER_PORTRAIT, + columns: { count: 2, gap: 48 }, + ...(bidi ? { bidi: true } : {}), + }, + }, + { paragraphs: ['Tail section, back to a single column'] }, + ], + { type: 'continuous', pageSize: PAGE_SIZES.LETTER_PORTRAIT }, + ); + const layout = await convertAndLayout(pmDoc, { pageSize: PAGE_SIZES.LETTER_PORTRAIT }); + // Only the multi-column section's own paragraphs; the tail section is single-column. + const columnised = layout.pages[0].fragments.filter( + (fragment) => fragment.blockId.endsWith('-paragraph') && paragraphIndex(fragment.blockId) < 8, + ); + return columnised.sort((a, b) => paragraphIndex(a.blockId) - paragraphIndex(b.blockId)); + }; + + const ltr = await balanced(false); + const rtl = await balanced(true); + + // Balancing actually engaged: the 8 paragraphs are split across both columns, not stacked in one. + const ltrXs = [...new Set(ltr.map((f) => Math.round(f.x)))]; + expect(ltrXs).toHaveLength(2); + const [leftX, rightX] = ltrXs.sort((a, b) => a - b); + + // LTR balances into the left column first; RTL into the right one. Same split, mirrored sides. + expect(ltr.map((f) => Math.round(f.x))).toEqual([leftX, leftX, leftX, leftX, rightX, rightX, rightX, rightX]); + expect(rtl.map((f) => Math.round(f.x))).toEqual([rightX, rightX, rightX, rightX, leftX, leftX, leftX, leftX]); + // Balancing must not disturb the vertical rhythm either. + expect(rtl.map((f) => Math.round(f.y))).toEqual(ltr.map((f) => Math.round(f.y))); + }); + + it('treats an explicitly disabled w:bidi as left to right', async () => { + // `` is the section opting out, not opting in. + const layout = await layoutTwoColumnSection({ ...TWO_COLUMN_SECTION, bidi: false }); + const { columnXs, indicesByX } = readFirstPage(layout); + + expect(indicesByX.get(columnXs[0])![0]).toBe(0); + }); + + it('carries the section direction onto the column layout, and only when columns exist', async () => { + const withColumns = pmToFlowBlocks( + createPMDocWithSections([{ paragraphs: ['a'] }], { ...TWO_COLUMN_SECTION, bidi: true }), + ); + expect(getSectionBreaks(withColumns.blocks).map((block) => block.columns)).toEqual([ + { count: 2, gap: 48, direction: 'rtl' }, + ]); + + // A single-column RTL section has no order to flip. The adapter must not invent a column layout + // for it, or an unstyled section would start to look like it carries explicit column properties. + const singleColumn = pmToFlowBlocks( + createPMDocWithSections([{ paragraphs: ['a'] }], { + type: 'nextPage', + pageSize: PAGE_SIZES.LETTER_PORTRAIT, + bidi: true, + }), + ); + expect(getSectionBreaks(singleColumn.blocks).map((block) => block.columns)).toEqual([undefined]); + }); +}); diff --git a/packages/layout-engine/tests/src/test-helpers/section-test-utils.ts b/packages/layout-engine/tests/src/test-helpers/section-test-utils.ts index d52cf554ca..33993e027c 100644 --- a/packages/layout-engine/tests/src/test-helpers/section-test-utils.ts +++ b/packages/layout-engine/tests/src/test-helpers/section-test-utils.ts @@ -21,6 +21,11 @@ export type TestSectionProps = { orientation?: 'portrait' | 'landscape'; pageSize?: { w: number; h: number }; columns?: { count: number; gap: number }; + /** + * Section page direction (`w:sectPr/w:bidi`). RTL puts the FIRST column against the right margin + * and fills right to left, the way Word lays out a Hebrew or Arabic multi-column section. + */ + bidi?: boolean; margins?: { header?: number; footer?: number }; /** Vertical alignment of content within the section's pages */ vAlign?: 'top' | 'center' | 'bottom' | 'both'; @@ -189,6 +194,16 @@ function createSectPrElements(sectionProps: TestSectionProps): Array): Record => asRecord(element.attributes); +/** + * Word ST_OnOff: a bare `` means ON, and only the explicit falsy spellings turn it off + * (ECMA-376 §22.9.2.7). Mirrors `parseOnOff` in the style engine. + */ +const ST_OFF = new Set(['0', 'false', 'off']); +const readOnOff = (attrs: Record): boolean => { + const raw = asString(attrs['w:val']); + return raw == null ? true : !ST_OFF.has(raw.trim().toLowerCase()); +}; + const readSectPr = (sectPr: unknown): Partial => { const elements = Array.isArray(asRecord(sectPr).elements) ? (asRecord(sectPr).elements as Record[]) : []; const out: Partial = {}; + // `w:bidi` and `w:cols` are siblings in any order, so the direction is collected here and applied + // to the column layout after the loop. + let pageIsRtl = false; for (const element of elements) { const name = asString(element.name); @@ -188,6 +201,11 @@ const readSectPr = (sectPr: unknown): Partial => { continue; } + if (name === 'w:bidi') { + pageIsRtl = readOnOff(attrs); + continue; + } + if (name === 'w:vAlign') { out.vAlign = asString(attrs['w:val']) as SectionBreakBlock['vAlign']; continue; @@ -203,6 +221,14 @@ const readSectPr = (sectPr: unknown): Partial => { } } + // Section `w:bidi` governs section-level chrome, and on the column axis it decides which side the + // FIRST column sits on (ECMA-376 §17.6.1). Only applied when the section actually declares + // columns: absent `w:cols` means a single column, which has no order to flip, and synthesising a + // layout here would make an unstyled section look like it carries explicit column properties. + if (pageIsRtl && out.columns) { + out.columns = { ...out.columns, direction: 'rtl' }; + } + return out; }; diff --git a/tests/consumer-typecheck/src/layout-rtl-column-direction.ts b/tests/consumer-typecheck/src/layout-rtl-column-direction.ts new file mode 100644 index 0000000000..0c5274f5d6 --- /dev/null +++ b/tests/consumer-typecheck/src/layout-rtl-column-direction.ts @@ -0,0 +1,34 @@ +import type { Layout } from 'superdoc'; + +// `Layout.columns` is a `ColumnLayout`, and the section page direction (`w:sectPr/w:bidi`) travels +// on it because column geometry is what the axis decides: which side the FIRST column sits on. +// The field is reachable from outside the package through this nested shape, so it is pinned here. + +type PublicColumnLayout = NonNullable; +type PublicColumnDirection = NonNullable; + +// Both literals a section can carry have to be assignable from outside the package. +const rtlSection: PublicColumnLayout = { count: 2, gap: 48, direction: 'rtl' }; +const ltrSection: PublicColumnLayout = { count: 2, gap: 48, direction: 'ltr' }; + +// Absent means LTR, so a consumer that never heard of the axis must still type-check. +const directionless: PublicColumnLayout = { count: 2, gap: 48 }; + +// The field is optional, and reading it back yields the same union — no widening to `string`. +declare const layout: Layout; +const readDirection: PublicColumnDirection | undefined = layout.columns?.direction; + +const rtl: PublicColumnDirection = 'rtl'; +const ltr: PublicColumnDirection = 'ltr'; + +// A consumer must be able to hand a value it read straight back into a layout it builds. +declare const observed: PublicColumnDirection; +const roundTripped: PublicColumnLayout = { count: 3, gap: 24, direction: observed }; + +void rtlSection; +void ltrSection; +void directionless; +void readDirection; +void rtl; +void ltr; +void roundTripped;