fix(rtl-tables): drop double-mirror for logical cell margins (SD-3134) (#3272)

* fix(rtl-tables): drop double-mirror for logical cell margins (SD-3134)

Two paths needed alignment with the borders fix from #3227:

1. getTableCellMargins (super-converter) only iterated left/right/top/bottom,
   so inline w:tcMar/w:start and w:end never projected to top-level
   cellMargins. Extend the iteration to add marginStart/marginEnd, preserving
   the OOXML logical names.

2. convertCellMarginsToPx (pm-adapter table-styles) still pre-swapped
   marginStart/marginEnd for RTL via an isRtlTable param. DomPainter already
   mirrors padding for bidiVisual tables, so the table-style hydration path
   was double-mirroring. Remove the pre-swap and the param; pm-adapter now
   stores logical start/end LTR-default. Same rule the borders fix encodes:
   importer/style-engine preserve OOXML logical names, pm-adapter maps
   logical to LTR-default physical, DomPainter performs the single visual
   RTL mirror.

Flip the bad RTL unit test that was locking in the swap. Unfixme the
behavior regression test at tests/behavior/tests/tables/
rtl-bidivisual-asymmetric.spec.ts.

Verified:
- pm-adapter tests: 201 pass (table-styles + borders + table)
- super-converter tests: 2889 pass
- Playwright RTL bidiVisual asymmetric spec: 12 pass across
  chromium/firefox/webkit (tcMar test now passes for real, no fixme)

* fix(rtl-tables): resolve tcMar precedence over table-level defaults (SD-3134)

Follow-up to the original fix in this PR. A second-round review caught a
case the original change still got wrong: when a table has tblCellMar
with physical w:left/w:right defaults and a cell has inline tcMar with
logical w:start/w:end, the cell-level exception was silently dropped.

Root cause: the importer stored physical defaults in result.left/right
and logical inline values in result.marginStart/marginEnd. Downstream
extractCellPadding gave physical precedence (padding.left was already
non-null), so the inline w:start/w:end never reached the painter.

Per ECMA-376 section 17.4.68: "This setting [tcMar], if present, shall
override the table cell margins from the table-level cell margins".

Rewrite getTableCellMargins to resolve precedence inside the importer:
cell-level inline (physical or logical) wins over table-level defaults
(physical or logical). Output is a single physical-shaped object with
LTR-default semantic; DomPainter still owns the visual RTL mirror.

Verified with a Word-native fixture (table-level w:tblCellMar w:left=40
and w:right=40 plus cell-level w:tcMar w:start=480 and w:end=60) loaded
in the dev app:
- before: paddingLeft=2.667, paddingRight=2.667 (table defaults; bug)
- after:  paddingLeft=32, paddingRight=4 (cell exception; correct)

Tests:
- pm-adapter: 213 pass
- super-converter: 2889 pass
- behavior spec rtl-bidivisual-asymmetric: 15 pass across chromium /
  firefox / webkit (new precedence test + 4 existing ones)

* refactor(rtl-tables): drop dead defensive branch + freshen test comment

Comment-audit follow-up on SD-3134:

- readMargin had a defensive branch for a nested { value: { value, type } }
  shape, but no super-converter or layout-engine translator emits that
  shape. Per CLAUDE.md (do not add error handling for scenarios that
  cannot happen) the branch and its vague "some translators emit" comment
  are removed.

- The regression note on the round-1 tcMar behavior test described the
  round-1 importer output (marginStart/marginEnd keys), which no longer
  matches the round-2 output (resolved physical left/right). Refreshed
  so future agents do not look for keys that no longer exist.

No behavior change. Tests still pass.

Author: caio-pizzol

packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts

@@ -330,7 +330,7 @@ describe('hydrateTableStyleAttrs', () => {
     expect(result?.cellPadding?.right).toBeCloseTo((200 / 1440) * 96);
   });
 
-  it('maps style marginStart/marginEnd to physical sides for RTL tables', () => {
+  it('keeps marginStart/marginEnd LTR-default for RTL tables (painter mirrors at paint)', () => {
     const styles: StylesDocumentProperties = {
       ...emptyStyles,
       styles: {
@@ -354,8 +354,11 @@ describe('hydrateTableStyleAttrs', () => {
     } as unknown as PMNode;
 
     const result = hydrateTableStyleAttrs(table, buildContext(styles));
-    // In RTL, start => right, end => left
-    expect(result?.cellPadding?.right).toBeCloseTo((100 / 1440) * 96);
-    expect(result?.cellPadding?.left).toBeCloseTo((200 / 1440) * 96);
+    // pm-adapter stores logical start/end LTR-default (start => left,
+    // end => right). DomPainter performs the single visual RTL mirror at
+    // paint time; pre-mirroring here would double-swap for bidiVisual
+    // tables. SD-3134.
+    expect(result?.cellPadding?.left).toBeCloseTo((100 / 1440) * 96);
+    expect(result?.cellPadding?.right).toBeCloseTo((200 / 1440) * 96);
   });
 });

packages/layout-engine/pm-adapter/src/converters/table-styles.ts

@@ -35,15 +35,14 @@ export const hydrateTableStyleAttrs = (
 ): TableStyleHydration | null => {
   const hydration: TableStyleHydration = {};
   const tableProps = (tableNode.attrs?.tableProperties ?? null) as Record<string, unknown> | null;
-  const isRtlTable = tableProps?.rightToLeft === true;
 
   // Collect inline values first, then merge with style-resolved values below.
   let inlineBorders: HydratedTableBorders | undefined;
   let inlinePadding: BoxSpacing | undefined;
 
   // 1. Inline properties (highest priority)
   if (tableProps) {
-    const padding = convertCellMarginsToPx(tableProps.cellMargins as Record<string, unknown>, isRtlTable);
+    const padding = convertCellMarginsToPx(tableProps.cellMargins as Record<string, unknown>);
     if (padding) inlinePadding = normalizeCellPaddingTopBottom(padding);
 
     if (tableProps.borders && typeof tableProps.borders === 'object') {
@@ -99,10 +98,7 @@ export const hydrateTableStyleAttrs = (
     }
 
     if (resolved.cellMargins) {
-      const stylePadding = convertCellMarginsToPx(
-        resolved.cellMargins as unknown as Record<string, unknown>,
-        isRtlTable,
-      );
+      const stylePadding = convertCellMarginsToPx(resolved.cellMargins as unknown as Record<string, unknown>);
       if (stylePadding) {
         const normalizedStylePadding = normalizeCellPaddingTopBottom(stylePadding);
         hydration.cellPadding = inlinePadding
@@ -163,11 +159,12 @@ const adjustBorderSize = (border: Record<string, unknown>): Record<string, unkno
   return size != null ? { ...border, size } : border;
 };
 
-const convertCellMarginsToPx = (margins: Record<string, unknown>, isRtlTable = false): BoxSpacing | undefined => {
+const convertCellMarginsToPx = (margins: Record<string, unknown>): BoxSpacing | undefined => {
   if (!margins || typeof margins !== 'object') return undefined;
   const spacing: BoxSpacing = {};
-  const startSide: keyof BoxSpacing = isRtlTable ? 'right' : 'left';
-  const endSide: keyof BoxSpacing = isRtlTable ? 'left' : 'right';
+  // LTR-default mapping. pm-adapter stores logical start/end as physical
+  // left/right; DomPainter is the single owner of the visual RTL mirror.
+  // SD-3134: pre-mirroring here was double-swapping for bidiVisual tables.
   const keyMap: Record<string, keyof BoxSpacing> = {
     top: 'top',
     bottom: 'bottom',
@@ -177,8 +174,8 @@ const convertCellMarginsToPx = (margins: Record<string, unknown>, isRtlTable = f
     marginBottom: 'bottom',
     marginLeft: 'left',
     marginRight: 'right',
-    marginStart: startSide,
-    marginEnd: endSide,
+    marginStart: 'left',
+    marginEnd: 'right',
   };
 
   Object.entries(margins).forEach(([key, value]) => {

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js

@@ -287,19 +287,52 @@ const markTableCellAsVMergeConsumed = (node) => {
  */
 const getTableCellMargins = (inlineMargins, referencedStyles) => {
   const { cellMargins = {} } = referencedStyles;
-  return ['left', 'right', 'top', 'bottom'].reduce((acc, direction) => {
-    const key = `margin${direction.charAt(0).toUpperCase() + direction.slice(1)}`;
-    const inlineValue = inlineMargins ? inlineMargins?.[key]?.value : null;
-    const styleValue = cellMargins ? cellMargins[key] : null;
-    if (inlineValue != null) {
-      acc[direction] = twipsToPixels(inlineValue);
-    } else if (styleValue == null) {
-      acc[direction] = undefined;
-    } else if (typeof styleValue === 'object') {
-      acc[direction] = twipsToPixels(styleValue.value);
+  // Read a margin value as twips. Accepts numeric or { value, type } shape.
+  // Returns null when the source key is missing or unreadable.
+  const readMargin = (source, key) => {
+    if (!source) return null;
+    const v = source[key];
+    if (v == null) return null;
+    if (typeof v === 'number') return v;
+    if (typeof v === 'object' && typeof v.value === 'number') return v.value;
+    return null;
+  };
+
+  // Per OOXML §17.4.68 (tcMar) + §17.4.42/41 (tblCellMar): cell-level inline
+  // overrides table-level defaults. SD-3134: resolve precedence here so
+  // downstream sees a single physical-shaped output regardless of whether
+  // the source used physical (w:left/w:right) or logical (w:start/w:end)
+  // names. LTR-default semantic - DomPainter mirrors for RTL.
+  const sides = [
+    { physical: 'top', physicalKey: 'marginTop', logicalKey: null },
+    { physical: 'bottom', physicalKey: 'marginBottom', logicalKey: null },
+    { physical: 'left', physicalKey: 'marginLeft', logicalKey: 'marginStart' },
+    { physical: 'right', physicalKey: 'marginRight', logicalKey: 'marginEnd' },
+  ];
+  const result = {};
+  for (const { physical, physicalKey, logicalKey } of sides) {
+    // Cell-level inline values win as a unit over any table-level default
+    // (physical OR logical inline beats physical OR logical style).
+    const inlinePhysical = readMargin(inlineMargins, physicalKey);
+    const inlineLogical = logicalKey ? readMargin(inlineMargins, logicalKey) : null;
+    if (inlinePhysical != null) {
+      result[physical] = twipsToPixels(inlinePhysical);
+      continue;
+    }
+    if (inlineLogical != null) {
+      result[physical] = twipsToPixels(inlineLogical);
+      continue;
+    }
+    const stylePhysical = readMargin(cellMargins, physicalKey);
+    const styleLogical = logicalKey ? readMargin(cellMargins, logicalKey) : null;
+    if (stylePhysical != null) {
+      result[physical] = twipsToPixels(stylePhysical);
+    } else if (styleLogical != null) {
+      result[physical] = twipsToPixels(styleLogical);
     } else {
-      acc[direction] = twipsToPixels(styleValue);
+      result[physical] = undefined;
     }
-    return acc;
-  }, {});
+  }
+
+  return result;
 };

tests/behavior/tests/tables/fixtures/ltr-tcmar-overrides-table-default.docx

@@ -97,46 +97,90 @@ test('RTL bidiVisual table with asymmetric tblBorders renders start on visual ri
 // tcMar start/end asymmetric (§17.4.68 + cell-padding mirror)
 // ----------------------------------------------------------------------------
 
-// AIDEV-NOTE: temporary - test.fixme until SD-2771 lands the style-engine
-// projection for tableCellProperties.cellMargins.marginStart/marginEnd to
-// top-level cellMargins. Today only top/bottom project to top-level (as px
-// numbers), so extractCellPadding misses the asymmetric start/end values
-// and the painter defaults both sides to ~4px. Fix lives in style-engine
-// or the projection step (pm-adapter and painter handle the correct shape
-// once they get it).
-test.fixme(
-  'RTL bidiVisual cell with asymmetric tcMar renders larger start padding on visual right',
-  async ({ superdoc }) => {
-    await superdoc.loadDocument(path.resolve(__dirname, 'fixtures/rtl-bidivisual-tcmar-asymmetric.docx'));
-    await superdoc.waitForStable();
-
-    // Fixture: first cell tcMar/start=480 twips (large, ~24px) end=60 twips (small, ~3px).
-    // tcMar (§17.4.68) start/end follow table direction (same governance as
-    // tblBorders/start/end per §17.4.33/12 and the leading-edge rule in
-    // §17.4.15: left for LTR tables, right for RTL tables). In a bidiVisual
-    // table, start lands on visual right, so paddingRight > paddingLeft.
-    const padding = await superdoc.page.evaluate(() => {
-      const fragment = document.querySelector('.superdoc-table-fragment');
-      if (!fragment) return null;
-      const cells = Array.from(fragment.children).filter((el) => (el as HTMLElement).style?.position === 'absolute');
-      const target = cells.find((cell) => cell.textContent?.includes('start-padding-large'));
-      if (!target) return null;
-      const cs = window.getComputedStyle(target);
-      return {
-        paddingLeft: parseFloat(cs.paddingLeft),
-        paddingRight: parseFloat(cs.paddingRight),
-      };
-    });
-
-    expect(padding).not.toBeNull();
-    if (!padding) return;
-
-    // Larger padding should be on the visual right (where start renders in RTL).
-    expect(padding.paddingRight).toBeGreaterThan(padding.paddingLeft);
-    // And the difference should be substantial (~21px between 480 twips and 60 twips).
-    expect(padding.paddingRight - padding.paddingLeft).toBeGreaterThan(10);
-  },
-);
+// Regression for SD-3134: getTableCellMargins resolves cell-level
+// w:tcMar/start/end against table-level defaults inside the importer
+// and outputs LTR-default physical sides. convertCellMarginsToPx no
+// longer pre-swaps for RTL. With those two fixes the painter is the
+// single owner of the visual mirror and asymmetric tcMar renders
+// Word-equivalent.
+test('RTL bidiVisual cell with asymmetric tcMar renders larger start padding on visual right', async ({ superdoc }) => {
+  await superdoc.loadDocument(path.resolve(__dirname, 'fixtures/rtl-bidivisual-tcmar-asymmetric.docx'));
+  await superdoc.waitForStable();
+
+  // Fixture: first cell tcMar/start=480 twips (large, ~24px) end=60 twips (small, ~3px).
+  // tcMar (§17.4.68) start/end follow table direction (same governance as
+  // tblBorders/start/end per §17.4.33/12 and the leading-edge rule in
+  // §17.4.15: left for LTR tables, right for RTL tables). In a bidiVisual
+  // table, start lands on visual right, so paddingRight > paddingLeft.
+  const padding = await superdoc.page.evaluate(() => {
+    const fragment = document.querySelector('.superdoc-table-fragment');
+    if (!fragment) return null;
+    const cells = Array.from(fragment.children).filter((el) => (el as HTMLElement).style?.position === 'absolute');
+    const target = cells.find((cell) => cell.textContent?.includes('start-padding-large'));
+    if (!target) return null;
+    const cs = window.getComputedStyle(target);
+    return {
+      paddingLeft: parseFloat(cs.paddingLeft),
+      paddingRight: parseFloat(cs.paddingRight),
+    };
+  });
+
+  expect(padding).not.toBeNull();
+  if (!padding) return;
+
+  // Larger padding should be on the visual right (where start renders in RTL).
+  expect(padding.paddingRight).toBeGreaterThan(padding.paddingLeft);
+  // And the difference should be substantial (~21px between 480 twips and 60 twips).
+  expect(padding.paddingRight - padding.paddingLeft).toBeGreaterThan(10);
+});
+
+// ----------------------------------------------------------------------------
+// tcMar overrides table-level cellMargins per §17.4.68
+// ----------------------------------------------------------------------------
+
+// Regression for SD-3134 round-2: when a table has tblCellMar with physical
+// left/right defaults AND a cell has inline tcMar with logical start/end,
+// the cell-level exception must override the table-level default per
+// §17.4.68 ("This setting, if present, shall override the table cell margins
+// from the table-level cell margins"). Earlier the importer kept both shapes
+// in cellMargins and extractCellPadding gave physical left/right precedence,
+// so the inline start/end values were silently dropped.
+test('LTR table with table-level marginLeft/Right defaults + cell-level tcMar/start/end uses the cell-level values', async ({
+  superdoc,
+}) => {
+  await superdoc.loadDocument(path.resolve(__dirname, 'fixtures/ltr-tcmar-overrides-table-default.docx'));
+  await superdoc.waitForStable();
+
+  // Fixture:
+  //  - Table tblPr/tblCellMar w:left=40 dxa and w:right=40 dxa (~2.7px each)
+  //  - Cell 1 tcPr/tcMar w:start=480 dxa (~32px) and w:end=60 dxa (~4px)
+  //  - Cell 2 has no inline tcMar (inherits the table defaults)
+  const padding = await superdoc.page.evaluate(() => {
+    const fragment = document.querySelector('.superdoc-table-fragment');
+    if (!fragment) return null;
+    const cells = Array.from(fragment.children).filter((el) => (el as HTMLElement).style?.position === 'absolute');
+    const exception = cells.find((c) => c.textContent?.includes('inline tcMar exception cell'));
+    const defaulted = cells.find((c) => c.textContent?.includes('table default cell'));
+    if (!exception || !defaulted) return null;
+    const csE = window.getComputedStyle(exception);
+    const csD = window.getComputedStyle(defaulted);
+    return {
+      exception: { paddingLeft: parseFloat(csE.paddingLeft), paddingRight: parseFloat(csE.paddingRight) },
+      defaulted: { paddingLeft: parseFloat(csD.paddingLeft), paddingRight: parseFloat(csD.paddingRight) },
+    };
+  });
+
+  expect(padding).not.toBeNull();
+  if (!padding) return;
+
+  // Cell with inline tcMar uses its own asymmetric exception values.
+  expect(padding.exception.paddingLeft).toBeGreaterThan(20);
+  expect(padding.exception.paddingRight).toBeLessThan(10);
+  expect(padding.exception.paddingLeft - padding.exception.paddingRight).toBeGreaterThan(20);
+  // Cell without inline tcMar inherits the table-level defaults (~2.7px).
+  expect(padding.defaulted.paddingLeft).toBeLessThan(5);
+  expect(padding.defaulted.paddingRight).toBeLessThan(5);
+});
 
 // ----------------------------------------------------------------------------
 // gridBefore / gridAfter (§17.4.14 + §17.4.15)