diff --git a/packages/layout-engine/contracts/src/column-layout.test.ts b/packages/layout-engine/contracts/src/column-layout.test.ts index 5aca69b3e8..c48a25c25e 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, }); }); }); @@ -399,3 +408,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..44294cfcbd 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,17 @@ 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. 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) => Math.max(0, value)) + : undefined; const width = widths.reduce((max, value) => Math.max(max, value), 0); @@ -176,6 +232,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 +244,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 +267,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 +309,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,6 +380,7 @@ 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) ); @@ -287,6 +405,9 @@ export function columnRenderLayoutsEqual(a?: ColumnLayout, b?: ColumnLayout): bo 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); 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..e49901ca8c 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.test.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.test.ts @@ -347,6 +347,79 @@ 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('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; diff --git a/packages/layout-engine/layout-engine/src/column-balancing.ts b/packages/layout-engine/layout-engine/src/column-balancing.ts index 7a5dcee3b9..6fe1dd848c 100644 --- a/packages/layout-engine/layout-engine/src/column-balancing.ts +++ b/packages/layout-engine/layout-engine/src/column-balancing.ts @@ -7,6 +7,7 @@ */ import { getColumnGeometry, getColumnX, hasGenuinelyUnequalExplicitColumnWidths } from '@superdoc/contracts'; +import type { BaseDirection } from '@superdoc/contracts'; // ============================================================================ // Types and Interfaces @@ -657,6 +658,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,12 +794,18 @@ 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 current column, then by y within each column. During + // unbalanced layout the paginator fills column 0 top-to-bottom, then column 1, etc. — so column + // order followed by y preserves the original sequence. + // + // Which way "column order" runs across the page is direction-relative. In an RTL section column 0 + // is the RIGHT one, so document order DESCENDS in x; sorting ascending there would feed the + // balancer the trailing column first and silently scramble the balanced page's reading order, + // since the balanced x/y are then written back onto the fragments in this order. + const columnOrder = + sectionColumns.direction === 'rtl' ? (a: number, b: number) => b - a : (a: number, b: number) => a - b; const ordered = [...sectionFragments].sort((a, b) => { - if (a.x !== b.x) return a.x - b.x; + if (a.x !== b.x) return columnOrder(a.x, b.x); return a.y - b.y; }); @@ -864,6 +881,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); diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index bc4ec340cf..692f670f17 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -5193,6 +5193,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..f20843c160 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; }; 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;