Merge pull request #3354 from superdoc-dev/caio-pizzol/SD-3171-fix-style-cascade-bidivisual

fix(direction): only honor inline w:bidiVisual for table visual direction (SD-3171)

Author: caio-pizzol

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

@@ -2527,7 +2527,7 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () =>
     expect((cellBlocks[0] as ParagraphBlock).runs[0].text).toBe('Inner DPO');
   });
 
-  describe('tableDirectionContext (SD-3138 Phase 1B)', () => {
+  describe('tableDirectionContext (SD-3138 Phase 1B + SD-3171 inline-only visual direction)', () => {
     const mockBlockIdGenerator: BlockIdGenerator = vi.fn((kind) => `test-${kind}`);
     const mockPositionMap: PositionMap = new Map();
     const mockParagraphConverter = vi.fn(() => [
@@ -2576,7 +2576,11 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () =>
       expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('rtl');
     });
 
-    it('style cascade rightToLeft=true produces visualDirection=rtl', () => {
+    // SD-3171: Word-parity contract. `w:bidiVisual` on a style does NOT visually
+    // flip cells - Word reports the table as wdTableDirectionLtr and renders
+    // cells in logical order despite the style cascade. SuperDoc must match.
+    // Style-cascade rightToLeft alone leaves visualDirection undefined.
+    it('style cascade rightToLeft=true alone leaves visualDirection undefined (SD-3171 Word-parity)', () => {
       const result = tableNodeToBlock(
         buildTableNode(undefined, 'RtlStyle'),
         mockBlockIdGenerator,
@@ -2590,10 +2594,14 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () =>
         mockParagraphConverter,
         contextWithStyle('RtlStyle', { rightToLeft: true }),
       ) as TableBlock;
-      expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('rtl');
+      expect(result?.attrs?.tableDirectionContext).toBeDefined();
+      expect(result?.attrs?.tableDirectionContext?.visualDirection).toBeUndefined();
     });
 
-    it('inline rightToLeft=false overrides style cascade rightToLeft=true (visualDirection=ltr)', () => {
+    // SD-3171: even when style says RTL, inline-false still produces ltr - the
+    // inline layer is the only source we consult for visualDirection, and
+    // explicit `false` is honored.
+    it('inline rightToLeft=false produces visualDirection=ltr (style cascade ignored)', () => {
       const result = tableNodeToBlock(
         buildTableNode({ rightToLeft: false }, 'RtlStyle'),
         mockBlockIdGenerator,
@@ -2610,11 +2618,10 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () =>
       expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('ltr');
     });
 
-    it('inline bidiVisual=false overrides style cascade rightToLeft=true (alias-mixed override)', () => {
+    it('inline bidiVisual=false produces visualDirection=ltr (alias normalized, style cascade ignored)', () => {
       // Importer normalizes w:bidiVisual to `rightToLeft` so this shape is rare
-      // in practice, but the resolver must treat the two aliases as one signal
-      // per layer or an inline-false override against a style-true silently
-      // resolves to RTL.
+      // in practice. SD-3171: style cascade is ignored regardless; the assertion
+      // is that inline `false` on the bidiVisual alias is still honored.
       const result = tableNodeToBlock(
         buildTableNode({ bidiVisual: false }, 'RtlStyle'),
         mockBlockIdGenerator,

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

@@ -55,7 +55,6 @@ import {
 import {
   TableProperties,
   resolveTableCellProperties,
-  resolveTableProperties,
   resolveExistingTableEffectiveStyleId,
   type TableInfo,
 } from '@superdoc/style-engine/ooxml';
@@ -1023,30 +1022,23 @@ export function tableNodeToBlock(
     tableAttrs.tableProperties = tableProperties as Record<string, unknown>;
   }
 
-  // SD-3138 Phase 1B: resolve the table direction context from cascade-resolved
-  // table properties so downstream consumers (painter, layout-engine, editor
-  // navigation) read a typed `TableDirectionContext` instead of inspecting raw
-  // tableProperties.rightToLeft. Inline `w:bidiVisual` wins over the style
-  // cascade; explicit `false` overrides a cascade `true` per §17.4.1 + §17.17.4
-  // (the resolver handles the explicit-false case via SD-3141).
-  const styleResolvedTableProps =
-    effectiveStyleId && converterContext?.translatedLinkedStyles
-      ? resolveTableProperties(effectiveStyleId, converterContext.translatedLinkedStyles)
-      : undefined;
-  // Normalize the rightToLeft / bidiVisual aliases to a single signal PER LAYER
-  // before layering inline over style. Otherwise an inline `bidiVisual: false`
-  // paired with a style `rightToLeft: true` would resolve RTL because the two
-  // aliases get layered independently (inline-false on bidiVisual loses to
-  // style-true on rightToLeft). The importer normalizes w:bidiVisual to
-  // `rightToLeft` so this matters most when style-engine emits raw OOXML keys.
+  // SD-3171 Word-parity: `w:bidiVisual` visually flips cell order ONLY when
+  // set inline on the table itself. Word does not visually flip when the only
+  // source of `bidiVisual` is a style (verified empirically: a table style
+  // carrying `w:bidiVisual` produces TableDirection=wdTableDirectionLtr and
+  // renders cells in logical order in Word). The model-level cascade still
+  // happens via style-engine; we just don't feed style-derived bidiVisual into
+  // the painter's visual-direction signal.
+  //
+  // Normalize the rightToLeft / bidiVisual aliases on the inline layer
+  // (importer normalizes w:bidiVisual to `rightToLeft`; aliases stay possible
+  // when style-engine emits raw OOXML keys). `??` treats null/undefined as
+  // missing but preserves an explicit `false`, so an inline
+  // `<w:bidiVisual w:val="0"/>` is honored.
   const inlineProps = rawTableProperties as { rightToLeft?: boolean; bidiVisual?: boolean } | undefined;
-  const styleProps = styleResolvedTableProps as { rightToLeft?: boolean; bidiVisual?: boolean } | undefined;
   const inlineVisual = inlineProps?.rightToLeft ?? inlineProps?.bidiVisual;
-  const styleVisual = styleProps?.rightToLeft ?? styleProps?.bidiVisual;
-  // `??` treats null/undefined as missing but preserves an explicit `false`,
-  // so an inline `<w:bidiVisual w:val="0"/>` correctly overrides a style true.
   const effectiveForDirection = {
-    rightToLeft: inlineVisual ?? styleVisual,
+    rightToLeft: inlineVisual,
   };
   const sectionContext = converterContext?.sectionDirectionContext ?? resolveSectionDirection(undefined);
   tableAttrs.tableDirectionContext = resolveTableDirection(effectiveForDirection, sectionContext);

tests/behavior/tests/tables/fixtures/rtl-inline-bidivisual.docx

@@ -0,0 +1,47 @@
+import { test, expect } from '../../fixtures/superdoc.js';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+test.use({ config: { toolbar: 'full', showSelection: true } });
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+
+// SD-3171 positive control: when `w:bidiVisual` is set INLINE on the table's
+// own `tblPr` (not via a style), Word visually flips the cell order. The
+// companion test rtl-style-derived-bidivisual.spec.ts pins the negative
+// control (style cascade does NOT visually flip).
+//
+// Verified empirically: opening this fixture in Word shows
+// `Document.Tables(1).TableDirection === 0` (wdTableDirectionRtl) and
+// renders cells visually right-to-left (logical first cell on visual right).
+//
+// Together these two tests prove the SD-3171 fix is direction-specific: the
+// inline path is preserved, the style-cascade path is what changed.
+
+test('table with inline bidiVisual flips cells visually (logical first on visual right)', async ({ superdoc }) => {
+  await superdoc.loadDocument(path.resolve(__dirname, 'fixtures/rtl-inline-bidivisual.docx'));
+  await superdoc.waitForStable();
+
+  // Fixture: 1x3 table, logical cells A B C, inline `w:bidiVisual`.
+  // Expected visual order (left to right): C B A.
+  const cellLayout = await superdoc.page.evaluate(() => {
+    const fragment = document.querySelector('.superdoc-table-fragment');
+    if (!fragment) return null;
+    const fragRect = fragment.getBoundingClientRect();
+    const cells = Array.from(fragment.children).filter((el) => (el as HTMLElement).style?.position === 'absolute');
+    if (cells.length === 0) return null;
+    return cells
+      .map((cell) => {
+        const rect = (cell as HTMLElement).getBoundingClientRect();
+        return { text: (cell.textContent ?? '').trim(), relLeft: rect.left - fragRect.left };
+      })
+      .filter((c) => c.text === 'A' || c.text === 'B' || c.text === 'C')
+      .sort((a, b) => a.relLeft - b.relLeft);
+  });
+
+  expect(cellLayout).not.toBeNull();
+  if (!cellLayout) return;
+
+  expect(cellLayout).toHaveLength(3);
+  // Inline bidiVisual produces visualDirection='rtl' → painter mirrors cells.
+  expect(cellLayout.map((c) => c.text)).toEqual(['C', 'B', 'A']);
+});

tests/behavior/tests/tables/rtl-inline-bidivisual.spec.ts

@@ -5,46 +5,38 @@ import { fileURLToPath } from 'node:url';
 test.use({ config: { toolbar: 'full', showSelection: true } });
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
-// SD-2767 Wave 3 coverage gap: a table whose `w:bidiVisual` comes from the
-// style cascade (NOT inline on the table itself). The fixture defines a
-// custom table style with `w:bidiVisual` set, and the table references that
-// style via `w:tblStyle` with no inline `w:bidiVisual` override.
+// SD-3171 Word-parity contract: `w:bidiVisual` visually flips cell order ONLY
+// when set inline on the table itself. Word does NOT visually flip cells when
+// the only source of `w:bidiVisual` is a style cascade.
 //
-// Per ECMA-376 §17.4.1, `w:bidiVisual` flips the visual order of cells:
-// logical first cell renders on the visual right, logical last on the
-// visual left. The style cascade must resolve this the same way as inline
-// `w:bidiVisual` would.
+// Verified empirically: opening this fixture in Word shows
+// `Document.Tables(1).TableDirection === 1` (wdTableDirectionLtr) and renders
+// cells in logical order A B C. SuperDoc must match.
 //
-// Word confirms `Document.Tables(1).TableDirection === 1` (wdTableDirectionRtl)
-// when opening this fixture, even though `document.xml` has no inline
-// `w:bidiVisual`. SuperDoc must match.
-
-test('RTL bidiVisual table inherited from a table style renders logical first cell on the visual right', async ({
-  superdoc,
-}) => {
+// History: PR #3350 originally asserted this fixture renders C B A (cells
+// visually flipped). That pinned SuperDoc's own pre-SD-3171 behavior, not
+// Word's. The flip came from pm-adapter's SD-3138 Phase 1B cascade path that
+// fell through from inline to style-resolved `bidiVisual`. SD-3171 removes
+// that fallback so style-cascade `bidiVisual` is ignored for visual direction.
+// The companion test rtl-inline-bidivisual.spec.ts pins the positive control
+// (inline `bidiVisual` still flips cells).
+
+test('table with style-derived bidiVisual renders cells in logical order (Word-parity)', async ({ superdoc }) => {
   await superdoc.loadDocument(path.resolve(__dirname, 'fixtures/rtl-style-derived-bidivisual.docx'));
   await superdoc.waitForStable();
 
   // Fixture: 1x3 table, logical cells A B C, style-set `bidiVisual`.
-  // Expected visual order (right to left): A B C.
-  // Expected visual order (left to right): C B A.
+  // Expected visual order (left to right): A B C.
   const cellLayout = await superdoc.page.evaluate(() => {
     const fragment = document.querySelector('.superdoc-table-fragment');
     if (!fragment) return null;
     const fragRect = fragment.getBoundingClientRect();
-
-    // Find all rendered cells. The painter positions cells absolutely within
-    // the fragment with left offsets that reflect their visual order.
     const cells = Array.from(fragment.children).filter((el) => (el as HTMLElement).style?.position === 'absolute');
     if (cells.length === 0) return null;
-
     return cells
       .map((cell) => {
         const rect = (cell as HTMLElement).getBoundingClientRect();
-        return {
-          text: (cell.textContent ?? '').trim(),
-          relLeft: rect.left - fragRect.left,
-        };
+        return { text: (cell.textContent ?? '').trim(), relLeft: rect.left - fragRect.left };
       })
       .filter((c) => c.text === 'A' || c.text === 'B' || c.text === 'C')
       .sort((a, b) => a.relLeft - b.relLeft);
@@ -53,11 +45,7 @@ test('RTL bidiVisual table inherited from a table style renders logical first ce
   expect(cellLayout).not.toBeNull();
   if (!cellLayout) return;
 
-  // Three cells found, in left-to-right visual order.
   expect(cellLayout).toHaveLength(3);
-
-  // RTL via style cascade: visual L-to-R order should be C, B, A.
-  // If the style cascade is direction-blind, cells would render A, B, C and
-  // this assertion would fail.
-  expect(cellLayout.map((c) => c.text)).toEqual(['C', 'B', 'A']);
+  // SD-3171: style-cascade `bidiVisual` does not visually flip cells. Match Word.
+  expect(cellLayout.map((c) => c.text)).toEqual(['A', 'B', 'C']);
 });