fix(layout): place the first column on the right in RTL sections
Part of #3952 — deliberately not Fixes, so merging this does not auto-close it.
This PR builds the axis: ColumnLayout gains a direction and the geometry mirrors on it. Nothing
populates that field yet. SectionDirectionContext.pageDirection has no producer either, and no
importer reads w:sectPr/w:bidi at all — the section-direction contract was specified and never
implemented on the producing side, which predates this PR. So a real Hebrew or Arabic document
still renders unchanged after this merges, and #3952 should stay open until an importer feeds the
axis. The evidence is in the contracts/src/index.ts thread below.
The problem
A section carrying w:sectPr/w:bidi fills its columns left to right, so the first paragraph of a two-column Hebrew section lands in the left column. Word puts it on the right (ECMA-376 §17.6.1).
SectionDirectionContext already documents pageDirection as governing "section-level chrome only (page numbers, columns, gutters)" — but ColumnGeometry measures x from the content-area left edge and no function in the column geometry ever received a direction. The axis was documented and never consumed.
It reads as a selection bug to users, because selection follows the fill order: drag from the column a Hebrew reader starts in (the right one) into the left one, and everything already highlighted below the anchor disappears. #3952 has the measurements.
The change
ColumnLayout gains an optional direction, and buildColumnGeometry mirrors the strip when it is 'rtl'.
Two decisions worth reviewing:
Mirror, don't reverse. index stays the FILL order and only the painted x moves. Every consumer that walks columns 0..n-1 keeps filling in document order, and since getColumnGeometry is the single source for positioning, fill, hit testing, separators, balancing, floating anchors and footnotes all follow from one place.
The mirror axis is the content area, not the strip. Explicit widths are deliberately not scaled to fill the content area, so a strip of explicit columns can underfill it. Mirroring about its own span would leave it pinned to the left margin and merely swap the columns inside it — the page would still read as left-aligned. NormalizedColumnLayout therefore carries the contentWidth it was normalized against; when absent, the mirror falls back to the span, which is exact whenever the columns fill the area (always so in equal mode). A single column is mirrored on the same rule: a no-op when it fills the area, right-aligned when an explicit one does not.
Five consumers that failed silently
None of these produce a type error when the axis is dropped, which is why each has a test that fails without its fix:
| Where | What went wrong |
|---|---|
getColumnAtX | Walked the geometry assuming x ascends with the index. |
balanceSectionOnPage | Reconstructed document order by sorting fragments on ascending x, on the premise that column 0 comes first left to right. Under RTL that premise inverts, so the balancer consumed the trailing column first and wrote the balanced x/y back in that order — scrambling a balanced page's reading order rather than mirroring it. Measured on a 2-column RTL page of 6 paragraphs: [432, 96, 96, 96, 432, 432] instead of [432, 432, 432, 96, 96, 96]. |
toBalancingColumns | Rebuilt the layout field by field and dropped the axis, so a balanced last page laid out left-to-right inside an otherwise right-to-left section. |
Footnote column attribution (incrementalLayout) | Broke on the first match under the same ascending assumption, so every fragment matched column 0: a page's notes collapsed into one group, the left column's notes printed under the right column, and its own note area stayed empty. |
Separator gate (DomPainter) | Read "content past the separator" as "content to the right", so a section whose content never left the first column drew a line Word does not draw. This one took two further passes to get right — see below. |
ColumnLayoutForAnchor and ParagraphAnchorsContext.columns now declare direction/contentWidth as well. Runtime was already correct — every caller passes a full normalized layout — but neither would have produced a type error if a future edit dropped the fields, which is exactly how toBalancingColumns went wrong.
Per-column gaps are also clamped to >= 0, matching the scalar gap beside them. OOXML cannot express a negative gutter (w:space is unsigned), but a hand-built layout can, and a gap negative enough to pull a column behind its predecessor would make an upright LTR strip answer hit tests as if it were mirrored.
The separator gate needed a second pass (b9e7e51)
The gate asks whether a later column holds content, because Word draws no line beside an empty
column. The first two attempts both answered it by comparing a fragment edge against the
separator x, picking whichever edge trails in the fill direction. No edge can answer that question.
Content wider than its column does not sit inside it, and resolveTableFrame places an over-wide
table at a negative offset from its column whenever the table is right-aligned or centred — and
end is the default justification for any bidiVisual table. So in an RTL section a wide table
starts left of its own column and ends past the separator, while never having left the later column:
both edges lie on the wrong side, and so does its origin. A negative w:ind does the same to a
paragraph, putting its origin in the gutter.
Measured on the repo's own fixture geometry (816px page, 96px margins, gap 48, separator at 408): a
wide right-aligned RTL table alone in the later column lands at x = -116, width = 500, and the
trailing-edge test suppressed a line Word draws. The earlier left-edge form had the mirror failure —
a page-anchored watermark at x = 0 satisfied it and drew a line Word does not draw.
The fix stops guessing from coordinates and reads fragment.columnIndex, the engine's own record of
the owning column, documented as the field to trust "when overflow crosses margins".
Reworked across 2438bd9, db5947e and f2a36d8, after three review rounds and a QA pass found
four defects in it. Two corrections to what earlier revisions of this description claimed:
The record is not the main path. columnIndex 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, which comes from a purely document-driven predicate with no
flag behind it). An earlier revision said paragraphs never carry one; that is false, and the claim
was load-bearing, so it is corrected here and in both code comments that repeated it. The conclusion
survives — a collapsed anchor carrier is a narrow shape, not the ordinary paragraph, so geometry
carries almost every fragment on the page.
Geometry needs four rules, not one. Containment of the origin and overlap are each wrong for a
case the other answers, because an indent and an over-wide box produce the same shape from opposite
causes. In order:
- Leading edge — a box starting on a column's own edge is that column's, fit or no fit. That is
ordinary content and content wider than its column, which overflows from that same edge.
Overlap alone gets it wrong once the columns are unequal enough for the spill to cover more of
the neighbour:widths: [100, 400], a 500px box at column 0's edge, 100px of its own column
against 352px of the next. - Trailing edge — a different question, not a mirror. 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]. - The origin, fit-checked — a smaller indent leaves both origin and box inside its own column.
The fit is what rejects a shifted origin: a centred orend-justified over-wide table also
begins inside an earlier column, and there the origin is no evidence at all. - Overlap, for an origin hung into a gutter by a negative
w:indor a float offset.
Before all four, a width bound: anything at least as wide as the area it would have to be content
of belongs to no column, which is what keeps page-anchored objects out of the gate. That area has
two bounds and the threshold is the smaller. The strip's own span, since explicit widths are not
scaled to fill the page 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
widths: [150, 600] with a 48px gap occupies 798px inside a 624px area, and against the strip bound
alone a page-wide graphic measures as merely partial and wins column 1 on overlap. Not
direction-specific: 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.
Two more at the call site. A float is excluded by identity, not by size — the width bound catches
a full-width watermark, but page.items is page.fragments.map(...) with no anchor filtering and an
anchored object carries its own measure.width, so a narrow float is the ordinary case: a 200px
logo at page x 500 on a 2-column page whose text never left column 0 had its origin inside column 1
and lit the gate. An out-of-range columnIndex is rejected rather than clamped — clamping turned
columnIndex: 5 on a two-column page into 1, which is exactly the "a later column holds content"
this gate asks about, invented out of a number describing no column on the page.
columnOwningSpan and balanceSectionOnPage's ordinalOf independently arrived at the same four
rules in the same order, differing only at the end, where a sort key must name a column and the
painter may answer null. They should be one shared helper in contracts with a documented rule
order; that is not in this PR.
findColumnContaining remains in contracts as the strict counterpart to getColumnAtX, which
must clamp because a click has to select something. Its spans are half-open so that columns authored
with no gutter (w:space="0") do not both claim the boundary they share — the boundary is exactly
where the later column's content begins, and an inclusive bound would hand it to the earlier column
in LTR but not in RTL, making the two directions disagree.
Backward compatibility
Column geometry and hit testing are byte-identical absent direction. Verified by running the pre-change column-layout.ts and this one side by side over 46,080 comparisons — 960 LTR configurations (1-4 columns; gaps 0/24/48/720; valid, invalid and surplus explicit widths; all three equalWidth states; separators; non-uniform per-column gaps) across 6 content widths from 0 to 5000. Zero differences. The one exception is the gaps clamp above, which differs only for a negative gutter -- something OOXML cannot express and only a hand-built layout can supply.
The separator gate is not neutral, and deliberately so. An earlier revision of this description said every path was unchanged for LTR; that is wrong, and worth stating plainly rather than leaving a reviewer to find it. The gate was wrong in both directions, so fixing it changes LTR output in three ways: a later column holding only an outdented paragraph now draws the rule Word draws and previously drew none; an anchored float no longer lights the gate for a column whose text never arrived; and an out-of-range recorded columnIndex no longer resolves to the last column. Each is a case the old edge test got wrong going right as well as going left, which is why the regression tests cover both directions.
Everything else on this branch is either behind a direction !== 'rtl' early return, or reduces to the previous expression when direction is absent: the balancing comparator is (a, b) => a - b for LTR, character-for-character the old sort, and the footnote-band helpers are the identity when w15:footnoteColumns is absent.
Tests
contracts 539 · layout-engine (bun) 968 · layout-bridge 1792 · painters/dom 1551. tsc, vp fmt --check and vp lint clean.
One failure in packages/layout-engine/tests, architecture-boundaries Guard E, flags paint-time DOM measurement in painters/dom/src/ruler/ruler-renderer.ts — an unmodified file that fails identically on a clean checkout of this base. Three scale tests (incrementalLayout.affected-frontier, incrementalLayout.checkpointMapEquality, persistent-page-surface) and one timing budget (incrementalLayout.semanticFlow) exceed their limits under worker contention on this machine and pass in isolation.
New coverage includes underfilling and overfilling explicit strips, separator mirroring onto the same physical gutter, hit testing at both edges and in the gutter, three columns with non-uniform per-column gaps, determineColumn under RTL (which had no coverage at all), and direction surviving clone/normalize/resolve and both equality checks.
For the separator gate specifically: a wide right-aligned RTL table whose x is outside its own
column, the same table belonging to the first column in both directions, a paragraph nudged into
the gutter by a negative indent, and a fragment on a zero-gap column boundary. Each was verified by
mutation — reverting the ownership lookup fails three of them, and restoring the inclusive span
bound fails the zero-gap one.
One open question is now settled. Whether Word treats the first <w:col> as the column you
start in (the rightmost, under RTL) or as the leftmost one visually decides whether unequal
authored widths need reversing in an RTL section. Tested against Word with a two-section file — an
RTL section and an LTR control, each w:num="2" w:equalWidth="0" with a narrow first column — the
narrow column renders on the right in the RTL section and on the left in the LTR control. The
first <w:col> is fill order, which is what this PR assumes, so no width reversal is needed.
On the "nothing sets direction" review finding
Correct, and worth stating precisely. tests/src/test-helpers/to-flow-blocks.ts now reads w:sectPr/w:bidi (ST_OnOff, so a bare element means on, applied after the element loop so sibling order with w:cols does not matter) and sets columns.direction. That makes section-breaks-rtl-columns.test.ts a real end-to-end check: paragraph 1 moves from x=72 to x=330 on a Letter page, and neutering the mirror fails it.
That is a test adapter, and it does not fix real documents. The production PM/OOXML→FlowBlock adapter lives in @superdoc/docx-engine, which is a plain npm dependency here — there is no sectPr parsing anywhere under packages/superdoc/src. SectionDirectionContext.pageDirection still has zero readers and zero writers.
So this branch is the complete engine half, proven end to end through an adapter that doubles as an exact reference for what the external one must do. Reaching users needs one more assignment wherever sectPr is projected onto SectionBreakBlock.columns, alongside the existing w:cols handling. Happy to write it if you point me at the seam, or to take a patch.
Word behaviour was checked against the OOXML spec and against Word's rendering of the same document; there is no pixel-diff gate to attach.