diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 9f701475b4..4bb8b39b33 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -760,6 +760,14 @@ export type TableRowAttrs = { value: number; rule?: 'auto' | 'atLeast' | 'exact' | string; }; + /** + * Row-level border override from OOXML `w:tblPrEx/w:tblBorders` (§17.4.61). + * Table property exceptions override the table-level borders for this row + * only. Rows without a `tblPrEx` border block leave this undefined and fall + * through to the table's borders. Resolved (eighth-points → px) by the v1 + * layout-adapter; the painter merges it over the table borders per edge. + */ + borders?: TableBorders; }; export type TableRow = { diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index 32c4be6e8b..f92963bcdf 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -5825,6 +5825,68 @@ describe('requirePageBoundary edge cases', () => { expect(pageContainsBlock(layout.pages[0], 'body')).toBe(true); }); + it('uses first ROW height (not full table) for a splittable table anchor so the chain starts and the table splits (SD-3345)', () => { + // A heading (keepNext) immediately followed by a tall splittable table. The + // heading + table FIRST ROW fits in the remaining space, but heading + the WHOLE + // table does not. Word starts the table here and splits it; reserving the full + // table height would push the heading + table wholly to the next page (large gap). + const filler: FlowBlock = { + kind: 'paragraph', + id: 'filler', + runs: [{ text: 'filler', fontFamily: 'Arial', fontSize: 12 }], + attrs: {}, + }; + const heading: FlowBlock = { + kind: 'paragraph', + id: 'heading', + runs: [{ text: 'Heading', fontFamily: 'Arial', fontSize: 24 }], + attrs: { keepNext: true }, + }; + const table = { + kind: 'table', + id: 'tbl', + rows: Array.from({ length: 4 }, (_unused, i) => ({ + id: `r${i}`, + cells: [ + { + id: `c${i}`, + blocks: [{ kind: 'paragraph', id: `p${i}`, runs: [{ text: 'x', fontFamily: 'Arial', fontSize: 10 }] }], + }, + ], + })), + } as unknown as TableBlock; + + const fillerMeasure: ParagraphMeasure = { kind: 'paragraph', lines: [makeLine(50)], totalHeight: 50 }; + const headingMeasure: ParagraphMeasure = { kind: 'paragraph', lines: [makeLine(20)], totalHeight: 20 }; + // 4 rows × 15px = 60px total; first row 15px. Cells carry a single measured line + // so the table-start preflight can render at least one row on the current page. + const tableMeasure = { + kind: 'table', + rows: Array.from({ length: 4 }, () => ({ + cells: [{ paragraph: { kind: 'paragraph', lines: [makeLine(15)], totalHeight: 15 }, width: 100, height: 15 }], + height: 15, + })), + columnWidths: [100], + totalWidth: 100, + totalHeight: 60, + } as unknown as TableMeasure; + + // Content area = 100px. After filler(50), 50px remain. + // - heading(20) + firstRow(15) = 35 <= 50 → chain fits → start on this page. + // - heading(20) + fullTable(60) = 80 > 50 → would advance without the fix. + // (80 <= 100 content height, so the blank-page guard does not suppress the advance.) + const options: LayoutOptions = { + pageSize: { w: 400, h: 160 }, + margins: { top: 30, right: 30, bottom: 30, left: 30 }, + }; + + const layout = layoutDocument([filler, heading, table], [fillerMeasure, headingMeasure, tableMeasure], options); + + // Heading and the table both start on page 0 (the table then splits across pages). + expect(pageContainsBlock(layout.pages[0], 'heading')).toBe(true); + expect(pageContainsBlock(layout.pages[0], 'tbl')).toBe(true); + }); + it('reclaims trailing spacing when both filler and chain starter have contextualSpacing', () => { // Both filler and chain starter have contextualSpacing + same style. // The trailing spacing should be reclaimed, making room for the chain. diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 89f356b7b3..b95195f7d7 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -438,13 +438,27 @@ function calculateChainHeight( totalHeight += interParagraphSpacing + anchorHeight; } else { - // Non-paragraph anchor (table, image, etc.): use full height - // No contextual spacing applies to non-paragraph blocks + // Non-paragraph anchor (table, image, etc.). + // No contextual spacing applies to non-paragraph blocks. // Skip anchored tables - they're positioned out of flow and don't consume flow height // (consistent with shouldSkipAnchoredTable guard in legacy keepNext path) const isAnchoredTable = anchorBlock.kind === 'table' && (anchorBlock as TableBlock).anchor?.isAnchored === true; if (!isAnchoredTable) { - totalHeight += prevSpacingAfter + getMeasureHeight(anchorBlock, anchorMeasure); + // For a table anchor, only require the FIRST ROW to stay with the chain, not + // the full table. The keepNext contract keeps the heading with the table's + // start; the table itself splits across pages (SD-3345). Reserving the full + // height pushed a heading + tall splittable table wholly to the next page, + // leaving a large gap, where Word starts the table here and splits it. This + // mirrors the paragraph anchor's first-line optimization (SD-1282). A table + // whose first row cannot split is still handled by the table-start preflight. + let anchorHeight = getMeasureHeight(anchorBlock, anchorMeasure); + if (anchorBlock.kind === 'table' && anchorMeasure.kind === 'table' && anchorMeasure.rows.length > 0) { + const firstRowHeight = anchorMeasure.rows[0]?.height; + if (typeof firstRowHeight === 'number' && Number.isFinite(firstRowHeight) && firstRowHeight > 0) { + anchorHeight = firstRowHeight; + } + } + totalHeight += prevSpacingAfter + anchorHeight; } } } diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.test.ts b/packages/layout-engine/painters/dom/src/table/border-utils.test.ts index 7d11407077..ebadd10b2d 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.test.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.test.ts @@ -22,6 +22,7 @@ import { hasExplicitCellBorders, swapTableBordersLR, swapCellBordersLR, + resolveBorderConflict, } from './border-utils.js'; describe('applyBorder', () => { @@ -522,3 +523,42 @@ describe('swapCellBordersLR', () => { expect(swapCellBordersLR(undefined)).toBeUndefined(); }); }); + +describe('resolveBorderConflict (ECMA-376 §17.4.66)', () => { + const D9 = { style: 'single' as const, width: 1.333, color: '#BDD7EE' }; + + it('collapses two identical borders to one (symmetric → no doubling)', () => { + // The M&A checklist case: adjacent cells both specify the same border. + const winner = resolveBorderConflict(D9, { ...D9 }); + expect(winner).toMatchObject({ style: 'single', color: '#BDD7EE' }); + }); + + it('keeps the present border when the opposing side is none (asymmetric → no dropped border)', () => { + // The it1007 case: header has a bottom border, the body cell below has no top. + const headerBottom = { style: 'single' as const, width: 1, color: '#000000' }; + expect(resolveBorderConflict(undefined, headerBottom)).toEqual(headerBottom); + expect(resolveBorderConflict({ style: 'none' }, headerBottom)).toEqual(headerBottom); + expect(resolveBorderConflict(headerBottom, undefined)).toEqual(headerBottom); + }); + + it('returns undefined when neither side has a border', () => { + expect(resolveBorderConflict(undefined, undefined)).toBeUndefined(); + expect(resolveBorderConflict({ style: 'none' }, { style: 'none' })).toBeUndefined(); + expect(resolveBorderConflict({ style: 'single', width: 0, color: '#000' }, undefined)).toBeUndefined(); + }); + + it('the heavier-weight border wins (double over single)', () => { + const single = { style: 'single' as const, width: 1, color: '#000000' }; + const dbl = { style: 'double' as const, width: 1, color: '#000000' }; + // weight: single = 1×1 = 1, double = 2×3 = 6 → double wins + expect(resolveBorderConflict(single, dbl)).toEqual(dbl); + expect(resolveBorderConflict(dbl, single)).toEqual(dbl); + }); + + it('on equal weight + identical style, the darker color wins', () => { + const dark = { style: 'single' as const, width: 1, color: '#000000' }; + const light = { style: 'single' as const, width: 1, color: '#FFFFFF' }; + // brightness(R+B+2G): dark=0 < light=1020 → dark wins + expect(resolveBorderConflict(light, dark)).toEqual(dark); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.ts b/packages/layout-engine/painters/dom/src/table/border-utils.ts index 6dee312c35..1285d0aae0 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.ts @@ -183,6 +183,102 @@ export const resolveTableBorderValue = ( return borderValueToSpec(fallback); }; +// Border "number" per ECMA-376 §17.4.66 (only the realistic styles; unknown → 1). +const BORDER_STYLE_NUMBER: Partial> = { + single: 1, + thick: 2, + double: 3, + dotted: 4, + dashed: 5, + dotDash: 6, + dotDotDash: 7, + triple: 8, + wave: 18, + doubleWave: 19, +}; +// Number of drawn lines per style (single=1, double=2, triple=3, …). +const BORDER_STYLE_LINES: Partial> = { + single: 1, + thick: 1, + double: 2, + dotted: 1, + dashed: 1, + dotDash: 1, + dotDotDash: 1, + triple: 3, + wave: 1, + doubleWave: 2, +}; + +export const isPresentBorder = (b?: BorderSpec): b is BorderSpec => + !!b && b.style !== undefined && b.style !== 'none' && (b.width === undefined || b.width > 0); + +/** + * True when a border is EXPLICITLY set to none/nil (`w:val="nil"`/`"none"`), as opposed to + * simply unset/absent. The distinction matters for shared interior edges (§17.4.66): an + * explicit none on BOTH adjacent cells suppresses the divider, while an unset side inherits + * the table's insideH/insideV. Accepts either a CellBorders BorderSpec (`{style:'none'}`) or + * a TableBorderValue (`{none:true}`). + */ +export const isExplicitNoneBorder = (b?: unknown): boolean => { + if (!b || typeof b !== 'object') return false; + const r = b as Record; + return r.style === 'none' || r.none === true; +}; + +const borderWeight = (b: BorderSpec): number => + (BORDER_STYLE_LINES[b.style as BorderStyle] ?? 1) * (BORDER_STYLE_NUMBER[b.style as BorderStyle] ?? 1); + +const colorBrightness = (color: string | undefined, formula: (r: number, g: number, bl: number) => number): number => { + const hex = (color ?? '#000000').replace('#', ''); + if (hex.length < 6) return 0; + const r = parseInt(hex.slice(0, 2), 16); + const g = parseInt(hex.slice(2, 4), 16); + const bl = parseInt(hex.slice(4, 6), 16); + return formula(r, g, bl); +}; + +/** + * OOXML cell-border conflict resolution (ECMA-376 §17.4.66). + * + * With zero cell spacing, two cells sharing an edge each specify a border; the spec + * collapses them to a SINGLE displayed border: + * 1. If either side is nil/none/absent, the opposing (present) border is displayed. + * 2. Otherwise the border with greater weight wins, where + * weight = (#lines in the style) × (style number). + * 3. Equal weight → the style higher on the precedence list (single first) wins. + * 4. Identical style → the color with the smaller brightness (R+B+2G, then B+2G, then + * G) wins; finally the first border (reading order) wins. + * + * @param a - One side's border (the owning cell's, e.g. the lower/right cell) + * @param b - The opposing side's border (e.g. the upper/left neighbor) + * @returns The single BorderSpec to display, or undefined if neither is present. + */ +export const resolveBorderConflict = (a?: BorderSpec, b?: BorderSpec): BorderSpec | undefined => { + const pa = isPresentBorder(a); + const pb = isPresentBorder(b); + if (!pa && !pb) return undefined; + if (!pa) return b; + if (!pb) return a; + const wa = borderWeight(a); + const wb = borderWeight(b); + if (wa !== wb) return wa > wb ? a : b; + const na = BORDER_STYLE_NUMBER[a.style as BorderStyle] ?? 99; + const nb = BORDER_STYLE_NUMBER[b.style as BorderStyle] ?? 99; + if (na !== nb) return na < nb ? a : b; + const formulas: Array<(r: number, g: number, bl: number) => number> = [ + (r, g, bl) => r + bl + 2 * g, + (_r, g, bl) => bl + 2 * g, + (_r, g) => g, + ]; + for (const f of formulas) { + const ba = colorBrightness(a.color, f); + const bb = colorBrightness(b.color, f); + if (ba !== bb) return ba < bb ? a : b; + } + return a; +}; + /** * Creates a border overlay element for a table fragment. * diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts index b77aa3edd4..79d5d1d529 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts @@ -577,6 +577,61 @@ describe('renderTableFragment', () => { }); }); + // Build a one-row measure whose single cell occupies only column 0, leaving + // the last grid column as an empty trailing gridAfter spacer. + const spacerMeasure = (spacerWidth: number): TableMeasure => ({ + kind: 'table', + rows: [ + { + cells: [ + { + paragraph: { kind: 'paragraph', lines: [], totalHeight: 20 }, + width: 100, + height: 20, + gridColumnStart: 0, + colSpan: 1, + }, + ], + height: 20, + }, + ], + columnWidths: [100, spacerWidth], + totalWidth: 100 + spacerWidth, + totalHeight: 20, + }); + + const renderSpacerTable = (spacerWidth: number): Element => + renderTableFragment({ + doc, + fragment: createTestTableFragment([ + { index: 0, x: 0, width: 100, minWidth: 25, resizable: true }, + { index: 1, x: 100, width: spacerWidth, minWidth: 25, resizable: true }, + ]), + context, + block: createTestTableBlock(), + measure: spacerMeasure(spacerWidth), + cellSpacingPx: 0, + effectiveColumnWidths: [100, spacerWidth], + renderLine: () => doc.createElement('div'), + applyFragmentFrame: () => {}, + applySdtDataset: () => {}, + applyStyles: () => {}, + }); + + it('omits the resize boundary for a degenerate trailing gridAfter spacer column (SD-3345)', () => { + // Spacer is narrower than its own min width → degenerate → its left-edge + // resize boundary is suppressed so it does not crowd the table-edge handle. + const parsed = JSON.parse(renderSpacerTable(7).getAttribute('data-table-boundaries')!); + expect(parsed.segments[1]).toEqual([]); + }); + + it('keeps the resize boundary for a normal-width trailing column (control)', () => { + // Same shape but the trailing column meets its min width → not degenerate → + // the boundary is kept. Proves the suppression is gated on degeneracy. + const parsed = JSON.parse(renderSpacerTable(40).getAttribute('data-table-boundaries')!); + expect(parsed.segments[1].length).toBeGreaterThan(0); + }); + it('should embed row boundary metadata when rowBoundaries are present', () => { const block = createTestTableBlock(); const measure = createTestTableMeasure(); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index 4a0faf4db7..9200c1ada9 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -313,6 +313,28 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // Track which column boundaries exist in this row const boundariesInRow = new Set(); + // Columns occupied by a cell in this row. Used to detect a trailing + // w:gridAfter spacer column this row leaves empty. + const occupiedCols = new Set(); + for (const cellMeasure of rowMeasure.cells) { + const s = cellMeasure.gridColumnStart ?? 0; + const sp = cellMeasure.colSpan ?? 1; + for (let c = s; c < s + sp; c++) occupiedCols.add(c); + } + // A degenerate trailing gridAfter spacer (last column, unoccupied this row, + // narrower than its own min width) sits a few px from the table edge. Emitting + // a resize boundary at its left edge crowds the table-edge handle and reads as a + // doubled border on hover, so skip that boundary for this row (SD-3345). + const lastColIndex = columnCount - 1; + const lastColMeta = fragment.metadata.columnBoundaries[lastColIndex]; + const skipTrailingSpacerBoundary = + lastColIndex > 0 && + !occupiedCols.has(lastColIndex) && + !!lastColMeta && + typeof lastColMeta.width === 'number' && + typeof lastColMeta.minWidth === 'number' && + lastColMeta.width < lastColMeta.minWidth; + for (const cellMeasure of rowMeasure.cells) { const startCol = cellMeasure.gridColumnStart ?? 0; const colSpan = cellMeasure.colSpan ?? 1; @@ -323,8 +345,10 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement if (startCol > 0) { boundariesInRow.add(startCol); } - // End boundary (right edge of cell) - if (endCol < columnCount) { + // End boundary (right edge of cell), unless it lands on a degenerate + // trailing gridAfter spacer (its left edge is the table edge for practical + // purposes, handled by the table-edge handle). + if (endCol < columnCount && !(skipTrailingSpacerBoundary && endCol === lastColIndex)) { boundariesInRow.add(endCol); } } @@ -411,6 +435,25 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement return r?.height ?? 0; }); + // Per-row rightmost occupied grid column (exclusive), INCLUDING cells that span into a row + // via w:vMerge (rowspan) from an earlier row. A single row's measure only lists the cells + // that START in that row, so on a rowspan-continuation row the columns held by a spanning + // cell look empty. The single-owner edge helpers (rowRightEdgeCol / nextRowMaxCol) would + // then undercount and treat a leftmost cell as the rightmost column (drawing an interior + // right border) or treat a covered column as a gridAfter gap (drawing an interior bottom), + // doubling the shared edge. Counting rowspan occupancy keeps those edges single-owned. + // (SD-1797) + const rowOccupiedRightCols: number[] = new Array(measure.rows.length).fill(0); + measure.rows.forEach((rowM, r) => { + for (const c of rowM?.cells ?? []) { + const right = (c.gridColumnStart ?? 0) + (c.colSpan ?? 1); + const lastRow = Math.min(measure.rows.length - 1, r + (c.rowSpan ?? 1) - 1); + for (let rr = r; rr <= lastRow; rr += 1) { + if (right > rowOccupiedRightCols[rr]) rowOccupiedRightCols[rr] = right; + } + } + }); + // First row starts after space before table content (space between table border and first row) let y = cellSpacingPx; @@ -428,6 +471,12 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement y, rowMeasure, row: block.rows[r], + prevRow: r > 0 ? block.rows[r - 1] : undefined, + prevRowMeasure: r > 0 ? measure.rows[r - 1] : undefined, + nextRow: r < block.rows.length - 1 ? block.rows[r + 1] : undefined, + nextRowMeasure: r < block.rows.length - 1 ? measure.rows[r + 1] : undefined, + rowOccupiedRightCol: rowOccupiedRightCols[r], + nextRowOccupiedRightCol: rowOccupiedRightCols[r + 1], totalRows: block.rows.length, tableBorders, columnWidths: effectiveColumnWidths, @@ -595,6 +644,12 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement y, rowMeasure, row: block.rows[r], + prevRow: r > 0 ? block.rows[r - 1] : undefined, + prevRowMeasure: r > 0 ? measure.rows[r - 1] : undefined, + nextRow: r < block.rows.length - 1 ? block.rows[r + 1] : undefined, + nextRowMeasure: r < block.rows.length - 1 ? measure.rows[r + 1] : undefined, + rowOccupiedRightCol: rowOccupiedRightCols[r], + nextRowOccupiedRightCol: rowOccupiedRightCols[r + 1], totalRows: block.rows.length, tableBorders, columnWidths: effectiveColumnWidths, diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts index 4c1661ced4..7062f0569b 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts @@ -126,7 +126,68 @@ describe('renderTableRow', () => { expect(call.borders?.right).toBeDefined(); }); - it('does not paint interior right border for explicit cell borders in collapsed mode', () => { + // SD-3028: a tblPrEx override on the row BELOW that suppresses the shared horizontal edge + // (insideH none/nil) means the lower cell — which owns that interior edge in the single-owner + // model — won't draw it. When the upper row's border comes from the table/style (no cell + // tcBorder for the neighbor path to pick up), the grid bottom would be dropped. §17.4.66 + // (present beats none) requires the upper row to close the grid by drawing its own insideH. + const collapsedStyleRow = (overrides: Record = {}) => + createDeps({ + rowIndex: 0, + totalRows: 6, + cellSpacingPx: 0, + ...overrides, + }); + const noneBorder = { none: true }; + const allNoneOverride = { + id: 'row-next', + attrs: { + borders: { + top: noneBorder, + bottom: noneBorder, + left: noneBorder, + right: noneBorder, + insideH: noneBorder, + insideV: noneBorder, + }, + }, + cells: [{ id: 'cell-next', blocks: [{ kind: 'paragraph', id: 'pn', runs: [] }] }], + }; + + it('closes the grid by drawing its own bottom when the next row suppresses the shared edge (tblPrEx none)', () => { + renderTableRow(collapsedStyleRow({ nextRow: allNoneOverride }) as never); + + expect(renderTableCellMock).toHaveBeenCalledTimes(1); + const call = getRenderedCellCall(); + expect(call.borders?.bottom).toBeDefined(); + expect((call.borders?.bottom as { style?: string })?.style).toBe('single'); + }); + + it('does not draw an interior bottom when the next row has no override (lower cell owns it, no doubling)', () => { + renderTableRow(collapsedStyleRow() as never); + + expect(renderTableCellMock).toHaveBeenCalledTimes(1); + const call = getRenderedCellCall(); + expect(call.borders?.bottom).toBeUndefined(); + }); + + it('does not draw an interior bottom when the next row override keeps a present shared edge', () => { + renderTableRow( + collapsedStyleRow({ + nextRow: { + id: 'row-next', + attrs: { borders: { insideH: { style: 'single', width: 1, color: '#D9D9D9' } } }, + cells: [{ id: 'cell-next', blocks: [{ kind: 'paragraph', id: 'pn', runs: [] }] }], + }, + }) as never, + ); + + expect(renderTableCellMock).toHaveBeenCalledTimes(1); + const call = getRenderedCellCall(); + expect(call.borders?.bottom).toBeUndefined(); + }); + + it('draws its own interior right border when the right neighbor is borderless (asymmetric, no doubling)', () => { renderTableRow( createDeps({ rowIndex: 0, @@ -165,12 +226,117 @@ describe('renderTableRow', () => { ); expect(renderTableCellMock).toHaveBeenCalledTimes(2); - const firstCall = renderTableCellMock.mock.calls[0][0] as { - borders?: { right?: unknown; left?: unknown }; - }; + const firstCall = renderTableCellMock.mock.calls[0][0] as { borders?: { right?: unknown; left?: unknown } }; + const secondCall = renderTableCellMock.mock.calls[1][0] as { borders?: { left?: unknown } }; - expect(firstCall.borders?.right).toBeUndefined(); + // Asymmetric edge: only cell-1 declares a right border (cell-2 is borderless), so cell-1 + // owns and paints it on itself; cell-2 does NOT redraw the shared edge, so it's drawn once. + expect(firstCall.borders?.right).toBeDefined(); expect(firstCall.borders?.left).toBeDefined(); + expect(secondCall.borders?.left).toBeUndefined(); + }); + + // SD-3028: "some borders" tables set a cell's shared vertical edge to w:val="nil" on BOTH + // sides to remove a divider, while the table style (TableGrid) defines insideV. The explicit + // nil on both cells must suppress the divider (§17.4.66); it must NOT fall back to insideV. + const twoCellRow = (cell0Borders: unknown, cell1Borders: unknown) => + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100, 100], + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + ], + }, + row: { + id: 'row-1', + cells: [ + { id: 'cell-1', attrs: { borders: cell0Borders }, blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }, + { id: 'cell-2', attrs: { borders: cell1Borders }, blocks: [{ kind: 'paragraph', id: 'p2', runs: [] }] }, + ], + }, + }); + const noneSpec = { style: 'none' as const, width: 0 }; + const bottomOnly = { + top: noneSpec, + right: noneSpec, + bottom: { style: 'single' as const, width: 1, color: '#000000' }, + left: noneSpec, + }; + + it('suppresses an interior vertical divider when BOTH adjacent cells set the shared edge to nil', () => { + renderTableRow(twoCellRow(bottomOnly, bottomOnly) as never); + + expect(renderTableCellMock).toHaveBeenCalledTimes(2); + const secondCall = renderTableCellMock.mock.calls[1][0] as { borders?: { left?: unknown } }; + expect(secondCall.borders?.left).toBeUndefined(); + }); + + it('still inherits the table insideV divider when only ONE side is explicitly nil (the other is unset)', () => { + // cell-2 left = nil, cell-1 right = unset (inherits insideV, present) -> §17.4.66 present wins. + renderTableRow(twoCellRow({ bottom: { style: 'single', width: 1, color: '#000000' } }, bottomOnly) as never); + + expect(renderTableCellMock).toHaveBeenCalledTimes(2); + const secondCall = renderTableCellMock.mock.calls[1][0] as { borders?: { left?: unknown } }; + expect(secondCall.borders?.left).toBeDefined(); + }); + + // SD-1797: a single row's measure only lists cells that START in it, so on a w:vMerge + // (rowspan) continuation row the columns held by a cell spanning from above look empty. + // `rowOccupiedRightCol` / `nextRowOccupiedRightCol` count that occupancy so the single-owner + // edge ownership doesn't misfire (a leftmost cell drawing a right border, or a covered column + // mistaken for a gridAfter gap) and double the shared edge. + const sparseRow = (overrides: Record = {}) => + createDeps({ + rowIndex: 2, + totalRows: 6, + cellSpacingPx: 0, + columnWidths: [100, 100, 100, 100], + rowMeasure: { height: 20, cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }] }, + row: { id: 'row-x', cells: [{ id: 'c', blocks: [{ kind: 'paragraph', id: 'p', runs: [] }] }] }, + ...overrides, + }); + + it('does not draw a right border on a leftmost cell of a rowspan-continuation row', () => { + // Columns 1-3 are covered by a vMerge cell spanning from above (rowOccupiedRightCol = 4), + // so this leftmost cell is NOT the rightmost column and must not draw insideV as its right. + renderTableRow(sparseRow({ rowOccupiedRightCol: 4 }) as never); + + const call = getRenderedCellCall(); + expect(call.borders?.right).toBeUndefined(); + expect(call.borders?.left).toBeDefined(); + }); + + it('does not treat a rowspan-covered column below a spanning cell as a gridAfter gap', () => { + // A cell spanning all 4 columns; the row below is fully covered (occupancy 4) -> no gap, + // so the spanning cell does NOT draw its own bottom (the cell below owns the shared edge). + renderTableRow( + sparseRow({ + rowMeasure: { height: 20, cells: [{ width: 400, height: 20, gridColumnStart: 0, colSpan: 4, rowSpan: 1 }] }, + nextRowOccupiedRightCol: 4, + }) as never, + ); + + const call = getRenderedCellCall(); + expect(call.borders?.bottom).toBeUndefined(); + }); + + it('still treats a genuine gridAfter gap as a bottom boundary (SD-3345 preserved)', () => { + // The cell spans all 4 columns but the row below only reaches column 2 (real gridAfter gap), + // so the spanning cell must draw its own bottom across the uncovered span. + renderTableRow( + sparseRow({ + rowMeasure: { height: 20, cells: [{ width: 400, height: 20, gridColumnStart: 0, colSpan: 4, rowSpan: 1 }] }, + nextRowOccupiedRightCol: 2, + }) as never, + ); + + const call = getRenderedCellCall(); + expect(call.borders?.bottom).toBeDefined(); }); it('does not paint interior bottom border for explicit cell borders in collapsed mode on non-final row', () => { @@ -228,6 +394,368 @@ describe('renderTableRow', () => { expect(call.borders?.bottom).toBeDefined(); }); + describe('row-level border override (w:tblPrEx)', () => { + const D9 = { style: 'single' as const, width: 1, color: '#D9D9D9' }; + const rowBorderOverride = { top: D9, right: D9, bottom: D9, left: D9, insideH: D9, insideV: D9 }; + + it('applies the row border override over the table borders for that row', () => { + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100], + rowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + row: { + id: 'row-1', + cells: [{ id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }], + attrs: { borders: rowBorderOverride }, + }, + }) as never, + ); + + const call = getRenderedCellCall(); + // Single cell touches all edges; the row override (#D9D9D9) wins over the + // default table borders (#000000) on every side. + expect(call.borders?.top).toEqual(D9); + expect(call.borders?.right).toEqual(D9); + expect(call.borders?.bottom).toEqual(D9); + expect(call.borders?.left).toEqual(D9); + }); + + it('draws the row override even when the table borders are explicitly none (FWC form rows)', () => { + const none = { none: true as const }; + renderTableRow( + createDeps({ + rowIndex: 1, + totalRows: 3, + cellSpacingPx: 0, + columnWidths: [100], + tableBorders: { top: none, right: none, bottom: none, left: none, insideH: none, insideV: none }, + rowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + row: { + id: 'row-1', + cells: [{ id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }], + attrs: { borders: rowBorderOverride }, + }, + }) as never, + ); + + const call = getRenderedCellCall(); + // Table says borderless, but this row's tblPrEx draws #D9D9D9. Top is an + // interior edge (row 1 of 3) → resolves from the override's insideH. + expect(call.borders?.top).toEqual(D9); + expect(call.borders?.left).toEqual(D9); + }); + + it('leaves table borders unchanged for a row without an override (callout row)', () => { + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100], + rowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + row: { + id: 'row-1', + cells: [{ id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }], + // no attrs.borders — falls through to the table borders + }, + }) as never, + ); + + const call = getRenderedCellCall(); + // Regression guard: absent a row override, the cell still paints the table + // border (#000000), not the #D9D9D9 override from the other tests. + expect(call.borders?.top).toEqual({ style: 'single', width: 1, color: '#000000' }); + }); + }); + + describe('trailing gridAfter columns (w:gridAfter)', () => { + it('draws the right border on the rightmost real cell when gridAfter pads the grid', () => { + // FWC form pattern: two content columns + one trailing gridAfter spacer column. + // The rightmost real cell (col 1) does not reach totalCols (3), but it is the + // row's right edge, so it must own the table/row right border (Word behaviour). + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100, 100, 20], // col 2 is the empty gridAfter spacer + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + ], + }, + row: { + id: 'row-1', + cells: [ + { id: 'c0', blocks: [{ kind: 'paragraph', id: 'p0', runs: [] }] }, + { id: 'c1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }, + ], + }, + }) as never, + ); + + expect(renderTableCellMock).toHaveBeenCalledTimes(2); + const rightCell = renderTableCellMock.mock.calls[1][0] as { borders?: { right?: unknown } }; + // The default table borders include a single right border; the rightmost + // real cell now owns it despite the trailing gridAfter column. + expect(rightCell.borders?.right).toBeDefined(); + }); + }); + + describe('collapsed cell-border conflict resolution (ECMA-376 §17.4.66, SD-3345)', () => { + const B = { style: 'single' as const, width: 1.333, color: '#BDD7EE' }; + const allSides = { top: B, right: B, bottom: B, left: B }; + + it('draws a shared interior vertical edge once (no doubling) for adjacent cells with identical borders', () => { + // The M&A checklist case: no table-level borders, every cell has all 4 sides. + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 2, + cellSpacingPx: 0, + columnWidths: [100, 100], + tableBorders: undefined, + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + ], + }, + row: { + id: 'r', + cells: [ + { id: 'c0', attrs: { borders: allSides }, blocks: [{ kind: 'paragraph', id: 'p0', runs: [] }] }, + { id: 'c1', attrs: { borders: allSides }, blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }, + ], + }, + }) as never, + ); + const left = renderTableCellMock.mock.calls[0][0] as { borders?: { right?: unknown } }; + const right = renderTableCellMock.mock.calls[1][0] as { borders?: { left?: unknown } }; + // Single-owner: the left cell's interior RIGHT is suppressed; the right cell draws + // the shared edge as its LEFT (the §17.4.66 winner). So the edge is drawn exactly once. + expect(left.borders?.right).toBeUndefined(); + expect(right.borders?.left).toBeDefined(); + }); + + it('keeps BOTH side borders on a cell whose neighbors are borderless (SD-3345 RTL start/end)', () => { + // The RTL tcBorders fixture in logical space: the first cell declares left (start) AND + // right (end) borders; the other cells are borderless. Both must stay on that cell — + // after the downstream RTL swap they become visual-right (start) and visual-left (end), + // not move onto a borderless neighbor. (Regression: single-owner used to delegate the + // interior right, dropping the cell's own end border.) + const start = { style: 'single' as const, width: 2, color: '#FF0000' }; + const end = { style: 'single' as const, width: 2, color: '#0000FF' }; + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100, 100, 100], + tableBorders: undefined, + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 2, colSpan: 1, rowSpan: 1 }, + ], + }, + row: { + id: 'r', + cells: [ + { + id: 'c0', + attrs: { borders: { left: start, right: end } }, + blocks: [{ kind: 'paragraph', id: 'p0', runs: [] }], + }, + { id: 'c1', attrs: {}, blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }, + { id: 'c2', attrs: {}, blocks: [{ kind: 'paragraph', id: 'p2', runs: [] }] }, + ], + }, + }) as never, + ); + const c0 = renderTableCellMock.mock.calls[0][0] as { borders?: { left?: unknown; right?: unknown } }; + expect(c0.borders?.left).toMatchObject({ color: '#FF0000' }); + expect(c0.borders?.right).toMatchObject({ color: '#0000FF' }); + }); + + it('keeps the above cell bottom border when this cell has no top (asymmetric → no dropped line)', () => { + // The it1007 regression case: header has a bottom border, the body cell below has no top. + const black = { style: 'single' as const, width: 1, color: '#000000' }; + renderTableRow( + createDeps({ + rowIndex: 1, + totalRows: 2, + cellSpacingPx: 0, + columnWidths: [100], + tableBorders: undefined, + rowMeasure: { height: 20, cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }] }, + row: { + id: 'r1', + // body cell: borders on left/right/bottom but NOT top + cells: [ + { + id: 'cb', + attrs: { borders: { left: black, right: black, bottom: black } }, + blocks: [{ kind: 'paragraph', id: 'pb', runs: [] }], + }, + ], + }, + prevRow: { + id: 'r0', + cells: [ + { + id: 'ch', + attrs: { borders: { top: black, left: black, right: black, bottom: black } }, + blocks: [{ kind: 'paragraph', id: 'ph', runs: [] }], + }, + ], + }, + prevRowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + }) as never, + ); + const cell = renderTableCellMock.mock.calls[0][0] as { borders?: { top?: unknown } }; + // This cell has no top border, but the cell above has a bottom border → §17.4.66 + // rule 1 keeps the present border, drawn here as this cell's top. + expect(cell.borders?.top).toMatchObject({ style: 'single', color: '#000000' }); + }); + + it('paints the above cell bottom border on a fully BORDERLESS row below it (SD-2969 clause-header)', () => { + // The SD-2969 case: a clause-header row has top+bottom borders; the row directly + // below it has NO border attribute at all. Single-owner gives the shared edge to + // the lower cell, so without the §17.4.66 fallback the header's bottom border is + // dropped entirely (the cell above suppressed its own bottom, the borderless cell + // below never drew it). + const black = { style: 'single' as const, width: 1, color: '#000000' }; + renderTableRow( + createDeps({ + rowIndex: 1, + totalRows: 2, + cellSpacingPx: 0, + columnWidths: [100], + tableBorders: undefined, + rowMeasure: { height: 8, cells: [{ width: 100, height: 8, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }] }, + row: { + id: 'r1', + // borderless cell: no `borders` attribute at all + cells: [{ id: 'cs', attrs: {}, blocks: [{ kind: 'paragraph', id: 'ps', runs: [] }] }], + }, + prevRow: { + id: 'r0', + cells: [ + { + id: 'ch', + attrs: { borders: { top: black, left: black, right: black, bottom: black } }, + blocks: [{ kind: 'paragraph', id: 'ph', runs: [] }], + }, + ], + }, + prevRowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + }) as never, + ); + const cell = renderTableCellMock.mock.calls[0][0] as { borders?: { top?: unknown } }; + expect(cell.borders?.top).toMatchObject({ style: 'single', color: '#000000' }); + }); + + it('draws its own bottom border when the next row leaves a gridAfter gap under it (SD-3345 callout corner)', () => { + // SD-3345 23_notification: the callout cell spans the full grid (gridSpan), but the + // row below has a gridAfter so its real cells do not reach the callout's rightmost + // column. Single-owner would defer the callout's bottom to the row below, which then + // stops short of the right edge → a gap at the bottom-right corner. The callout must + // draw its own bottom across the uncovered span instead. + const blue = { style: 'single' as const, width: 1, color: '#342D8C' }; + renderTableRow( + createDeps({ + rowIndex: 0, + totalRows: 2, + cellSpacingPx: 0, + columnWidths: [100, 100], + tableBorders: undefined, + // full-width callout cell (spans both columns) + rowMeasure: { height: 20, cells: [{ width: 200, height: 20, gridColumnStart: 0, colSpan: 2, rowSpan: 1 }] }, + row: { + id: 'r0', + cells: [ + { + id: 'callout', + attrs: { borders: { top: blue, left: blue, right: blue, bottom: blue } }, + blocks: [{ kind: 'paragraph', id: 'p0', runs: [] }], + }, + ], + }, + // next row covers only col0 (col1 is a gridAfter spacer → not a real cell) + nextRowMeasure: { + height: 20, + cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }], + }, + }) as never, + ); + const cell = renderTableCellMock.mock.calls[0][0] as { borders?: { bottom?: unknown } }; + expect(cell.borders?.bottom).toMatchObject({ style: 'single', color: '#342D8C' }); + }); + + it('suppresses this row top border when the cell above spans past it (gridAfter) so the edge is drawn once', () => { + // The companion to the case above: when the spanning cell owns the shared bottom edge, + // the narrower row below must NOT also draw its top, or the two adjacent cell divs stack + // into a doubled line (this painter has no border-collapse). (SD-3345 callout) + const blue = { style: 'single' as const, width: 1, color: '#342D8C' }; + renderTableRow( + createDeps({ + rowIndex: 1, + totalRows: 2, + cellSpacingPx: 0, + columnWidths: [100, 100], + tableBorders: undefined, + // this row covers only col0 (col1 is its gridAfter spacer) + rowMeasure: { height: 20, cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }] }, + row: { + id: 'r1', + cells: [{ id: 'opt', attrs: {}, blocks: [{ kind: 'paragraph', id: 'po', runs: [] }] }], + }, + // the cell above spans BOTH columns and has a bottom border (it owns the shared edge) + prevRow: { + id: 'r0', + cells: [ + { + id: 'callout', + attrs: { borders: { top: blue, left: blue, right: blue, bottom: blue } }, + blocks: [{ kind: 'paragraph', id: 'p0', runs: [] }], + }, + ], + }, + prevRowMeasure: { + height: 20, + cells: [{ width: 200, height: 20, gridColumnStart: 0, colSpan: 2, rowSpan: 1 }], + }, + }) as never, + ); + const cell = renderTableCellMock.mock.calls[0][0] as { borders?: { top?: unknown } } | undefined; + expect(cell?.borders?.top).toBeUndefined(); + }); + }); + describe('RTL table (isRtl)', () => { it('mirrors cell x positions so first logical column is on the right', () => { renderTableRow( diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index d3b2a117f2..6db797374d 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -14,7 +14,10 @@ import { resolveTableCellBorders, borderValueToSpec, resolveTableBorderValue, + resolveBorderConflict, hasExplicitCellBorders, + isPresentBorder, + isExplicitNoneBorder, swapCellBordersLR, } from './border-utils.js'; import { getTableCellGridBounds, type TableCellGridPosition } from './grid-geometry.js'; @@ -32,6 +35,32 @@ type CellBorderResolutionArgs = { cellSpacingPx: number; continuesFromPrev: boolean; continuesOnNext: boolean; + /** Borders of the cell directly above (previous row, same grid column) for §17.4.66 conflict resolution. */ + aboveCellBorders?: CellBorders; + /** Borders of the cell directly to the left (same row, previous grid column). */ + leftCellBorders?: CellBorders; + /** Borders of the cell directly to the right (same row, next grid column), for asymmetric-edge ownership. */ + rightCellBorders?: CellBorders; + /** + * True when the next row's real cells do not reach this cell's right edge (e.g. the next + * row has a `w:gridAfter` spacer while this cell spans into it). The cell below then can't + * own the shared bottom edge across the uncovered span, so this cell must draw its own + * bottom border or the line stops short at the bottom-right corner. (SD-3345) + */ + nextRowLeavesRightGap?: boolean; + /** + * True when the cell ABOVE spans past this cell's row right edge (this row has a gridAfter + * relative to it). The spanning cell owns the shared bottom edge and draws it, so this cell + * must suppress its top border to avoid a doubled line. (SD-3345) + */ + deferTopToAboveCell?: boolean; + /** + * True when the row BELOW has a tblPrEx border override that suppresses its shared horizontal + * edge (insideH none/nil). The lower cell owns that edge but won't draw it, so a present + * table/style border on THIS row must be drawn here to close the grid (§17.4.61/§17.4.66). + * (SD-3028) + */ + nextRowSuppressesSharedTop?: boolean; }; const hasAnyResolvedBorder = (borders: CellBorders): boolean => @@ -52,14 +81,90 @@ const resolveRenderedCellBorders = ({ cellSpacingPx, continuesFromPrev, continuesOnNext, + aboveCellBorders, + leftCellBorders, + rightCellBorders, + nextRowLeavesRightGap, + deferTopToAboveCell, + nextRowSuppressesSharedTop, }: CellBorderResolutionArgs): CellBorders | undefined => { const hasExplicitBorders = hasExplicitCellBorders(cellBorders); + const cellBounds = getTableCellGridBounds(cellPosition); + const touchesTopBoundary = cellBounds.touchesTopEdge || continuesFromPrev; + // The bottom is a real boundary either when this is the last row / a fragment break, OR + // when the next row's real cells don't reach this cell's right edge (a gridAfter spacer + // under a spanning cell): the row below can't own the shared edge across the uncovered + // span, so this (spanning) cell owns and draws its full-width bottom. The row below then + // suppresses its top there (see `deferTopToAboveCell`) so the edge is drawn exactly once — + // this painter has no border-collapse, so two cells drawing it would stack into a doubled + // line, not overlap. (SD-3345) + const touchesBottomBoundary = cellBounds.touchesBottomEdge || continuesOnNext || nextRowLeavesRightGap === true; + + // A shared interior edge in the collapsed model is owned by the lower/right cell, so a + // border defined ONLY by the neighbor above/left must still be painted here — even when + // this cell has no border of its own — or the line is dropped entirely (the neighbor + // suppressed its own edge under single-owner). (SD-2969: a bordered clause-header row + // above a fully borderless spacer row.) + const hasInteriorNeighborBorder = + (!touchesTopBoundary && !deferTopToAboveCell && isPresentBorder(aboveCellBorders?.bottom)) || + (!cellBounds.touchesLeftEdge && isPresentBorder(leftCellBorders?.right)); + + // Collapsed model (zero cell spacing): single-owner positioning, where the value at a + // shared interior edge is the ECMA-376 §17.4.66 winner of the two adjacent cell borders. + // This draws a shared edge exactly ONCE (no doubling) while keeping the present border on + // an asymmetric edge (no dropped line). Runs whenever this cell OR a neighbor above/left + // defines a border, so `cb` defaults to {} for the borderless case (resolveBorderConflict + // (undefined, x) === x). Interior right/bottom are owned by the neighbor to the right/below; + // outer edges use the cell border (which beats the table border), falling back to the table + // border. Works whether or not table-level borders exist. (SD-3345, SD-2969) + if (cellSpacingPx === 0 && (hasExplicitBorders || hasInteriorNeighborBorder)) { + const cb = (cellBorders ?? {}) as CellBorders; + return { + top: touchesTopBoundary + ? resolveTableBorderValue(cb.top, tableBorders?.top) + : deferTopToAboveCell + ? undefined + : (resolveBorderConflict(cb.top, aboveCellBorders?.bottom) ?? + // Both sides not present: an explicit nil on BOTH adjacent cells suppresses the + // shared horizontal edge (§17.4.66); only inherit the table insideH when at least + // one side is merely unset. (SD-3028) + (isExplicitNoneBorder(cb.top) && isExplicitNoneBorder(aboveCellBorders?.bottom) + ? undefined + : borderValueToSpec(tableBorders?.insideH))), + // Vertical interior edges: when BOTH adjacent cells declare a border, the right cell + // owns it (draws its left as the §17.4.66 winner) so the edge is painted once (no + // doubling). When only ONE side declares a border (asymmetric, no doubling risk) that + // cell draws it on ITS OWN side — so an RTL cell's end (logical-right) border stays on + // the cell after the left/right swap instead of moving onto a borderless neighbor. (SD-3345) + left: cellBounds.touchesLeftEdge + ? resolveTableBorderValue(cb.left, tableBorders?.left) + : isPresentBorder(cb.left) + ? (resolveBorderConflict(cb.left, leftCellBorders?.right) ?? borderValueToSpec(tableBorders?.insideV)) + : isPresentBorder(leftCellBorders?.right) + ? undefined + : // Both sides not present: an explicit nil on BOTH adjacent cells suppresses the + // divider (§17.4.66); only fall back to the table insideV when at least one side + // is merely unset (and would inherit it). (SD-3028) + isExplicitNoneBorder(cb.left) && isExplicitNoneBorder(leftCellBorders?.right) + ? undefined + : borderValueToSpec(tableBorders?.insideV), + right: cellBounds.touchesRightEdge + ? resolveTableBorderValue(cb.right, tableBorders?.right) + : isPresentBorder(cb.right) && !isPresentBorder(rightCellBorders?.left) + ? cb.right + : undefined, + bottom: touchesBottomBoundary ? resolveTableBorderValue(cb.bottom, tableBorders?.bottom) : undefined, + }; + } + if (hasBordersAttribute && !hasExplicitBorders) { return undefined; } if (!tableBorders) { + // Separate mode (non-zero cell spacing) with explicit borders, or no table borders + // at all: there is no shared-edge conflict, so draw every specified border. return hasExplicitBorders ? { top: cellBorders.top, @@ -70,27 +175,8 @@ const resolveRenderedCellBorders = ({ : undefined; } - const cellBounds = getTableCellGridBounds(cellPosition); - const touchesTopBoundary = cellBounds.touchesTopEdge || continuesFromPrev; - const touchesBottomBoundary = cellBounds.touchesBottomEdge || continuesOnNext; - if (hasExplicitBorders) { - if (cellSpacingPx === 0) { - // Collapsed model: avoid double interior borders by using single-owner sides. - // Keep explicit top/left (or table fallbacks), and only render right/bottom on table edges. - // Assumes shared interior edges specify the same border on both adjacent cells (e.g. Google Docs - // round-trips this way). Asymmetric (only one cell’s side set) may miss a line until we add conflict resolution. - return { - top: resolveTableBorderValue(cellBorders.top, touchesTopBoundary ? tableBorders.top : tableBorders.insideH), - right: cellBounds.touchesRightEdge ? resolveTableBorderValue(cellBorders.right, tableBorders.right) : undefined, - bottom: touchesBottomBoundary ? resolveTableBorderValue(cellBorders.bottom, tableBorders.bottom) : undefined, - left: resolveTableBorderValue( - cellBorders.left, - cellBounds.touchesLeftEdge ? tableBorders.left : tableBorders.insideV, - ), - }; - } - + // Separate mode (cellSpacingPx > 0) with table-level borders present. return { top: resolveTableBorderValue(cellBorders.top, touchesTopBoundary ? tableBorders.top : tableBorders.insideH), right: resolveTableBorderValue(cellBorders.right, cellBounds.touchesRightEdge ? tableBorders.right : undefined), @@ -115,10 +201,16 @@ const resolveRenderedCellBorders = ({ const baseBorders = resolveTableCellBorders(tableBorders, cellPosition); + // The row below owns this interior bottom edge, but if its tblPrEx override suppresses it + // (insideH none), draw this row's own present interior horizontal border so the grid still + // closes. (SD-3028) + const insideHSpec = borderValueToSpec(tableBorders.insideH); + const interiorBottom = nextRowSuppressesSharedTop && isPresentBorder(insideHSpec) ? insideHSpec : baseBorders.bottom; + return { top: touchesTopBoundary ? borderValueToSpec(tableBorders.top) : baseBorders.top, right: baseBorders.right, - bottom: touchesBottomBoundary ? borderValueToSpec(tableBorders.bottom) : baseBorders.bottom, + bottom: touchesBottomBoundary ? borderValueToSpec(tableBorders.bottom) : interiorBottom, left: baseBorders.left, }; }; @@ -142,6 +234,24 @@ type TableRowRenderDependencies = { rowMeasure: TableRowMeasure; /** Row data (cells, attributes), or undefined for empty rows */ row?: TableRow; + /** Previous (above) row data + measure, for collapsed-border conflict resolution (§17.4.66). */ + prevRow?: TableRow; + prevRowMeasure?: TableRowMeasure; + /** Next (below) row data, to detect a row-level border override that suppresses the shared + * horizontal edge so the current row closes the grid itself (§17.4.61/§17.4.66). */ + nextRow?: TableRow; + /** Next (below) row measure, to detect a gridAfter gap under a spanning cell (SD-3345). */ + nextRowMeasure?: TableRowMeasure; + /** + * Rightmost occupied grid column (exclusive) for THIS row, counting cells that span into it + * via w:vMerge (rowspan) from an earlier row. Falls back to this row's own cells when absent. + * Prevents a leftmost cell on a rowspan-continuation row from being treated as the rightmost + * column. (SD-1797) + */ + rowOccupiedRightCol?: number; + /** Same as {@link rowOccupiedRightCol} for the NEXT row, so a rowspan continuation below is + * not mistaken for a gridAfter gap (which would double the shared bottom edge). (SD-1797) */ + nextRowOccupiedRightCol?: number; /** Total number of rows in the table (for border resolution) */ totalRows: number; /** Table-level borders (for resolving cell borders) */ @@ -254,6 +364,12 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { y, rowMeasure, row, + prevRow, + prevRowMeasure, + nextRow, + nextRowMeasure, + rowOccupiedRightCol, + nextRowOccupiedRightCol, totalRows, tableBorders, columnWidths, @@ -279,6 +395,45 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { const totalCols = columnWidths.length; + // Effective right grid edge for THIS row's border ownership. A row with a + // trailing w:gridAfter reserves empty columns past its last cell (FWC forms do + // this), so the rightmost real cell never reaches `totalCols` and the + // single-owner model would drop the row's right border. Word draws the right + // border at the rightmost cell, treating gridAfter columns as outside the box. + // Use the last occupied column as the right edge; for rows without gridAfter + // this equals totalCols (no change). + // Prefer the rowspan-aware occupied width (counts cells spanning into this row via vMerge); + // fall back to this row's own cells when the caller doesn't provide it. (SD-1797, SD-3345) + const rowRightEdgeCol = + rowOccupiedRightCol != null && rowOccupiedRightCol > 0 + ? Math.min(totalCols, rowOccupiedRightCol) + : rowMeasure.cells.length + ? Math.min(totalCols, Math.max(...rowMeasure.cells.map((c) => (c.gridColumnStart ?? 0) + (c.colSpan ?? 1)))) + : totalCols; + + // Row-level border override (OOXML w:tblPrEx/w:tblBorders, §17.4.61). When this + // row carries its own borders, they override the table borders for this row only, + // merged per edge so unspecified sides still inherit the table. Rows without an + // override paint with the table borders unchanged (no behavior change). + const rowBorderOverride = row?.attrs?.borders; + const effectiveTableBorders: TableBorders | undefined = rowBorderOverride + ? { ...(tableBorders ?? {}), ...rowBorderOverride } + : tableBorders; + + // When the NEXT row carries a tblPrEx override that suppresses its shared horizontal edge + // (insideH = none/nil), the lower cell — which owns that edge in the single-owner model — + // won't draw it, and a table/style-derived border above (no cell tcBorder for the SD-2969 + // neighbor path to pick up) would be dropped. Per §17.4.66 a present border beats the + // none, so THIS row must close the grid by drawing its own interior bottom. Gated on the + // next row actually having an override, so unoverridden tables are unchanged (no doubling). + // (SD-3028) + const nextRowBorderOverride = nextRow?.attrs?.borders; + const nextRowEffectiveInsideH = nextRowBorderOverride + ? ({ ...(tableBorders ?? {}), ...nextRowBorderOverride } as TableBorders).insideH + : undefined; + const nextRowSuppressesSharedTop = + nextRowBorderOverride !== undefined && !isPresentBorder(borderValueToSpec(nextRowEffectiveInsideH)); + /** * Calculates the horizontal position (x-coordinate) for a cell based on its grid column index. * @@ -359,6 +514,49 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { return width; }; + // Find the borders of the cell in `cells` that occupies grid column `gridCol`, using + // the row's measure to map cell index → grid span (handles colspan). Used to fetch the + // above/left neighbor's borders for §17.4.66 collapsed-border conflict resolution. + const findCellBordersAtColumn = ( + cells: TableRow['cells'] | undefined, + measureCells: TableRowMeasure['cells'] | undefined, + gridCol: number, + ): CellBorders | undefined => { + if (!cells || !measureCells) return undefined; + for (let i = 0; i < measureCells.length; i++) { + const start = measureCells[i].gridColumnStart ?? i; + const span = measureCells[i].colSpan ?? 1; + if (gridCol >= start && gridCol < start + span) return cells[i]?.attrs?.borders; + } + return undefined; + }; + + // Right edge (exclusive grid column) of the cell occupying `gridCol` in `measureCells`. + const findCellRightEdgeAtColumn = ( + measureCells: TableRowMeasure['cells'] | undefined, + gridCol: number, + ): number | undefined => { + if (!measureCells) return undefined; + for (let i = 0; i < measureCells.length; i++) { + const start = measureCells[i].gridColumnStart ?? i; + const span = measureCells[i].colSpan ?? 1; + if (gridCol >= start && gridCol < start + span) return start + span; + } + return undefined; + }; + + // Rightmost grid column (exclusive) covered by the next row's REAL cells. When a spanning + // cell's right edge exceeds this, the next row has a gridAfter spacer beneath it and can't + // own the shared bottom edge across the uncovered span. (SD-3345) + // Rowspan-aware occupied width of the next row (counts cells spanning into it); fall back to + // the next row's own cells. A covered column must not look like a gridAfter gap. (SD-1797) + const nextRowMaxCol = + nextRowOccupiedRightCol != null && nextRowOccupiedRightCol > 0 + ? nextRowOccupiedRightCol + : nextRowMeasure?.cells?.length + ? Math.max(...nextRowMeasure.cells.map((c) => (c.gridColumnStart ?? 0) + (c.colSpan ?? 1))) + : Infinity; + for (let cellIndex = 0; cellIndex < rowMeasure.cells.length; cellIndex += 1) { const cellMeasure = rowMeasure.cells[cellIndex]; const cell = row?.cells?.[cellIndex]; @@ -381,9 +579,27 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { gridColumnStart, colSpan, totalRows, - totalCols, + // Use the row's effective right edge so the rightmost cell owns the right + // border even when trailing w:gridAfter columns pad the grid (§17.4.55). + totalCols: rowRightEdgeCol, }; + // Neighbor borders for §17.4.66 collapsed-border conflict resolution: the cell above + // (previous row, same grid column) and the cell to the left (same row, previous column). + const aboveCellBorders = findCellBordersAtColumn(prevRow?.cells, prevRowMeasure?.cells, gridColumnStart); + const leftCellBorders = + gridColumnStart > 0 ? findCellBordersAtColumn(row?.cells, rowMeasure.cells, gridColumnStart - 1) : undefined; + // The cell to the right (same row, the column just past this cell's span) — used to keep + // an asymmetric vertical edge on the owning cell instead of moving it to the neighbor. + const rightCellBorders = findCellBordersAtColumn(row?.cells, rowMeasure.cells, gridColumnStart + colSpan); + // This cell spans past the next row's real cells (gridAfter spacer beneath its right edge). + const nextRowLeavesRightGap = gridColumnStart + colSpan > nextRowMaxCol; + // Conversely, the cell ABOVE spans past THIS row's right edge (this row has a gridAfter + // relative to it). The spanning cell then owns the full shared edge and draws its own + // bottom, so this cell must NOT also draw its top, or the edge doubles. (SD-3345) + const aboveCellRightEdge = findCellRightEdgeAtColumn(prevRowMeasure?.cells, gridColumnStart); + const deferTopToAboveCell = aboveCellRightEdge !== undefined && aboveCellRightEdge > rowRightEdgeCol; + // Resolve borders using logical positions, then swap output for RTL. // The resolver uses touchesLeftEdge/touchesRightEdge which are LOGICAL edges. // For RTL, logical left = visual right, so we swap the resolved CSS properties @@ -391,11 +607,17 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { const resolvedBorders = resolveRenderedCellBorders({ cellBorders: cellBordersAttr, hasBordersAttribute, - tableBorders, + tableBorders: effectiveTableBorders, cellPosition, cellSpacingPx, continuesFromPrev: continuesFromPrev === true, continuesOnNext: continuesOnNext === true, + aboveCellBorders, + leftCellBorders, + rightCellBorders, + nextRowLeavesRightGap, + deferTopToAboveCell, + nextRowSuppressesSharedTop, }); // RTL: swap resolved left↔right so CSS properties match visual edges const finalBorders = isRtl && resolvedBorders ? swapCellBordersLR(resolvedBorders) : resolvedBorders; diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts index 4765298947..27b9e32471 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts @@ -179,6 +179,64 @@ describe('table converter', () => { expect(result.rows[0].cells[0].paragraph.kind).toBe('paragraph'); }); + it('resolves w:tblPrEx row borders onto the row attrs, leaving rows without them untouched', () => { + // FWC form pattern (SD-3345): the table declares tblBorders=none, but form + // rows carry a w:tblPrEx/w:tblBorders override (#D9D9D9). The converter stores + // it raw (eighth-points) under tableRowProperties.tblPrExBorders; the adapter + // must resolve it to typed row.attrs.borders. The callout row has no override. + const D9 = { val: 'single', color: '#D9D9D9', themeColor: 'background1', themeShade: 'D9', size: 4, space: 0 }; + const node: PMNode = { + type: 'table', + attrs: { borders: {} }, // table-level borders are none/empty + content: [ + { + type: 'tableRow', + attrs: { tableRowProperties: { someFlag: true } }, // callout row: no tblPrExBorders + content: [ + { type: 'tableCell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'callout' }] }] }, + ], + }, + { + type: 'tableRow', + attrs: { + tableRowProperties: { + tblPrExBorders: { top: D9, left: D9, bottom: D9, right: D9, insideH: D9, insideV: D9 }, + }, + }, + content: [ + { + type: 'tableCell', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'First name(s)' }] }], + }, + ], + }, + ], + }; + + const result = tableNodeToBlock( + node, + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + + // Callout row: no tblPrEx → no resolved row borders (falls through to table). + expect(result.rows[0].attrs?.borders).toBeUndefined(); + + // Form row: tblPrEx resolved to typed borders (#D9D9D9, eighth-points → px). + const rowBorders = result.rows[1].attrs?.borders; + expect(rowBorders).toBeDefined(); + expect(rowBorders?.top).toMatchObject({ style: 'single', color: '#D9D9D9' }); + expect((rowBorders?.top as { width: number }).width).toBeGreaterThan(0); + expect(rowBorders?.insideH).toMatchObject({ style: 'single', color: '#D9D9D9' }); + }); + it('does not emit imported gridBefore/gridAfter placeholder cells into TableBlock rows', () => { const node: PMNode = { type: 'table', diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts index f084386e84..b6d2d44e41 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts @@ -45,6 +45,7 @@ import { } from '../attributes/index.js'; import { pickNumber, twipsToPx } from '../utilities.js'; import { hydrateTableStyleAttrs } from './table-styles.js'; +import { resolveShadingFillColor } from '@converter/helpers.js'; import { collectTrackedChangeFromMarks } from '../marks/index.js'; import { annotateBlockWithTrackedChange, shouldHideTrackedNode } from '../tracked-changes.js'; import { @@ -308,18 +309,21 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { } } } - // Fall back to resolved style shading if no inline background + // Fall back to resolved style shading if no inline background. Resolve the fill + // base (explicit hex → theme fill), then apply the OOXML shading pattern + // (pctNN/solid) via the shared resolver so a table style's `pct10`/`auto` shading + // (e.g. a tblStylePr firstRow band) becomes the gray Word paints, instead of being + // dropped. (SD-2969) if (!cellBackgroundColor && resolvedTcProps?.shading) { - const { fill, themeFill, themeFillTint, themeFillShade } = resolvedTcProps.shading; - const normalizedFill = normalizeShadingColor(fill); - if (normalizedFill) { - cellBackgroundColor = normalizedFill; - } else if (themeFill && context.themeColors) { - const resolved = resolveThemeColorValue(themeFill, themeFillTint, themeFillShade, context.themeColors); - const normalizedTheme = normalizeShadingColor(resolved); - if (normalizedTheme) { - cellBackgroundColor = normalizedTheme; - } + const { fill, color, val, themeFill, themeFillTint, themeFillShade } = resolvedTcProps.shading; + let fillBase = normalizeShadingColor(fill); + if (!fillBase && themeFill && context.themeColors) { + const resolvedTheme = resolveThemeColorValue(themeFill, themeFillTint, themeFillShade, context.themeColors); + fillBase = normalizeShadingColor(resolvedTheme); + } + const resolved = resolveShadingFillColor({ val, color, fill: fillBase ?? fill }); + if (resolved) { + cellBackgroundColor = resolved.startsWith('#') ? resolved : `#${resolved}`; } } @@ -716,11 +720,23 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { const rowProps = rowNode.attrs?.tableRowProperties; const rowHeight = normalizeRowHeight(rowProps as Record | undefined); + + // Row-level border override from w:tblPrEx/w:tblBorders (ECMA-376 §17.4.61). + // The converter stores these raw (eighth-points) under tableRowProperties.tblPrExBorders. + // tblPrEx overrides the table's borders for this row only; rows without it fall + // through to the table borders (callout rows in FWC forms stay borderless this way). + const tblPrExBordersRaw = (rowProps as Record | undefined)?.tblPrExBorders; + const rowBorders = + tblPrExBordersRaw && typeof tblPrExBordersRaw === 'object' + ? extractTableBorders(tblPrExBordersRaw as Record, { unit: 'eighthPoints' }) + : undefined; + const attrs: TableRowAttrs | undefined = rowProps && typeof rowProps === 'object' ? { tableRowProperties: rowProps as Record, ...(rowHeight ? { rowHeight } : {}), + ...(rowBorders ? { borders: rowBorders } : {}), } : rowHeight ? { rowHeight } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/helpers.d.ts b/packages/super-editor/src/editors/v1/core/super-converter/helpers.d.ts index f72c7f1804..5b99926369 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/helpers.d.ts +++ b/packages/super-editor/src/editors/v1/core/super-converter/helpers.d.ts @@ -41,6 +41,9 @@ export function getDocxHighlightKeywordFromHex(hexColor: any): any; export function normalizeHexColor(hex: any): any; export function isValidHexColor(color: any): boolean; export function rgbToHex(rgb: any): string; +export function resolveShadingFillColor( + shading: { val?: string; color?: string; fill?: string } | null | undefined, +): string | null; export function ptToTwips(pt: any): number; export function twipsToPt(twips: any): number; export function getLineHeightValueString( diff --git a/packages/super-editor/src/editors/v1/core/super-converter/helpers.js b/packages/super-editor/src/editors/v1/core/super-converter/helpers.js index a18381b3d8..641085bc2c 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/helpers.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/helpers.js @@ -519,6 +519,7 @@ const rgbToHex = (rgb) => { }; const DEFAULT_SHADING_FOREGROUND_COLOR = '#000000'; +const DEFAULT_SHADING_FILL_COLOR = '#FFFFFF'; const hexToRgb = (hex) => { const normalized = normalizeHexColor(hex); @@ -549,21 +550,53 @@ const blendHexColors = (backgroundHex, foregroundHex, foregroundRatio) => { return `${toByte(r)}${toByte(g)}${toByte(b)}`; }; +/** + * Resolve an OOXML shading element (CT_Shd, ECMA-376 §17.3.5) to a concrete CSS + * hex fill WITHOUT a leading '#', or null when the shading produces no background. + * + * The shading model paints a pattern (`w:val`) of the foreground color (`w:color`) + * over the fill background (`w:fill`). The `auto` sentinel resolves to white for a + * fill and black for a foreground, so `pct10` + `color="auto"` + `fill="auto"` is + * 10% black over white ≈ light gray (#E6E6E6) — which is what Word renders. + * + * Theme colors (themeFill/themeColor) are resolved by callers that own the theme + * palette and passed back in as explicit hex; this helper handles explicit hex and + * the `auto` sentinel only. + * + * @param {{ val?: string, color?: string, fill?: string } | null | undefined} shading + * @returns {string | null} Hex without '#', or null for "no background". + */ const resolveShadingFillColor = (shading) => { if (!shading || typeof shading !== 'object') return null; - const fill = normalizeHexColor(shading.fill); - if (!fill) return null; - const val = typeof shading.val === 'string' ? shading.val.trim().toLowerCase() : ''; + if (val === 'nil' || val === 'none') return null; + + // The `auto` fill sentinel means "automatic background" (white). A missing or + // non-hex fill yields no explicit color. + const fillIsAuto = typeof shading.fill === 'string' && shading.fill.trim().toLowerCase() === 'auto'; + const fillHex = fillIsAuto ? null : normalizeHexColor(shading.fill); + const pctMatch = val.match(/^pct(\d{1,3})$/); - if (!pctMatch) return fill; + const isPattern = Boolean(pctMatch) || val === 'solid'; + + // No pattern (clear / unspecified): the fill IS the background. An automatic or + // missing fill therefore means "no background". + if (!isPattern) return fillHex; + + // Pattern (pctNN or solid): blend the foreground over the fill base. An automatic + // fill base resolves to white; an automatic or absent foreground resolves to black. + const baseHex = fillHex ?? DEFAULT_SHADING_FILL_COLOR; + const colorIsAuto = + typeof shading.color !== 'string' || shading.color.trim() === '' || shading.color.trim().toLowerCase() === 'auto'; + const foregroundHex = colorIsAuto + ? DEFAULT_SHADING_FOREGROUND_COLOR + : (normalizeHexColor(shading.color) ?? DEFAULT_SHADING_FOREGROUND_COLOR); - const pct = Number.parseInt(pctMatch[1], 10); - if (!Number.isFinite(pct) || pct < 0 || pct > 100) return fill; + const pct = pctMatch ? Number.parseInt(pctMatch[1], 10) : 100; // solid = 100% foreground + if (!Number.isFinite(pct) || pct < 0 || pct > 100) return fillHex; - const foreground = normalizeHexColor(shading.color) ?? DEFAULT_SHADING_FOREGROUND_COLOR; - return blendHexColors(fill, foreground, pct / 100) ?? fill; + return blendHexColors(baseHex, foregroundHex, pct / 100) ?? fillHex; }; const getLineHeightValueString = (lineHeight, defaultUnit, lineRule = '', isObject = false) => { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/helpers.test.js b/packages/super-editor/src/editors/v1/core/super-converter/helpers.test.js index e86cdab2e5..6ea1d23b5a 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/helpers.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/helpers.test.js @@ -10,6 +10,7 @@ import { dataUriToArrayBuffer, detectImageType, eighthPointsToPixels, + resolveShadingFillColor, } from './helpers.js'; import { getFallbackImageNameFromDataUri } from './helpers/mediaHelpers.js'; @@ -601,3 +602,43 @@ describe('eighthPointsToPixels', () => { }); }); }); + +describe('resolveShadingFillColor', () => { + it('returns null when there is no shading', () => { + expect(resolveShadingFillColor(null)).toBeNull(); + expect(resolveShadingFillColor(undefined)).toBeNull(); + }); + + it('returns the explicit fill for a clear pattern (no blend)', () => { + expect(resolveShadingFillColor({ val: 'clear', color: 'auto', fill: 'DCE6F1' })).toBe('DCE6F1'); + }); + + it('treats an automatic fill as no background when there is no pattern', () => { + expect(resolveShadingFillColor({ val: 'clear', color: 'auto', fill: 'auto' })).toBeNull(); + expect(resolveShadingFillColor({ fill: 'auto' })).toBeNull(); + }); + + it('returns null for an explicit no-shading pattern (nil/none)', () => { + expect(resolveShadingFillColor({ val: 'nil', fill: 'DCE6F1' })).toBeNull(); + expect(resolveShadingFillColor({ val: 'none', fill: 'DCE6F1' })).toBeNull(); + }); + + // SD-2969: a pattern over the automatic sentinel must resolve to gray (Word renders it gray), not garbage/white. + it('resolves pct10 over auto fill and color to light gray', () => { + // 10% black painted over automatic (white) ≈ #E6E6E6 + expect(resolveShadingFillColor({ val: 'pct10', color: 'auto', fill: 'auto' })).toBe('E6E6E6'); + }); + + it('resolves pct35 over explicit black-on-white to mid gray', () => { + expect(resolveShadingFillColor({ val: 'pct35', color: '000000', fill: 'FFFFFF' })).toBe('A6A6A6'); + }); + + it('blends a percentage pattern over an explicit fill and color', () => { + // 50% black over white = #808080 + expect(resolveShadingFillColor({ val: 'pct50', color: '000000', fill: 'FFFFFF' })).toBe('808080'); + }); + + it('treats a solid pattern as 100% foreground color', () => { + expect(resolveShadingFillColor({ val: 'solid', color: 'FF0000', fill: 'FFFFFF' })).toBe('FF0000'); + }); +});