From 96826099094dfb0b010c705cebd29c49474f87d7 Mon Sep 17 00:00:00 2001 From: nazlo90 Date: Fri, 15 May 2026 00:42:00 +0300 Subject: [PATCH] =?UTF-8?q?fix(super-converter):=20fix=20table-cell=20shad?= =?UTF-8?q?ing=20export=20=E2=80=94=20strip=20#=20prefix=20and=20emit=20va?= =?UTF-8?q?l=3D"clear"=20(SD-3142)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OOXML requires w:fill to be a bare 6-char hex string and w:val="clear" for solid fills. The exporter was emitting the color as-is (including a leading #), and omitting w:val entirely — both invalid per ECMA-376 §17.4.32. LibreOffice interprets a missing val as "nil" and renders the cell black regardless of the fill value. Two changes in the export path: - `translate-table-cell.js`: strips the # prefix via normalizeHexColor before writing w:fill, adds val:"clear" to the shading object, and guards against non-hex values (e.g. "auto") so they don't produce invalid markup. - `helpers.js` (resolveShadingFillColor): adds an isValidHexColor guard so that w:fill="auto" — a sentinel that Word uses to mean "no fill" — returns null instead of the string "AUTO", preventing it from being surfaced as a color downstream. 11 new tests across helpers.test.js and translate-table-cell.test.js; corrected an existing wrong assertion (w:fill was expected as '#FF00FF'; OOXML requires 'FF00FF'). All 12827 existing tests continue to pass. Not fixed in this PR: the import side of SD-3142 — cells with a table-style-inherited background currently render as white in SuperDoc's layout engine because the style cascade resolution (style-engine) doesn't yet propagate the conditional-format shading fill through to the painted cell. That path requires changes in style-engine and pm-adapter and is left for a follow-up. --- .../v1/core/super-converter/helpers.js | 2 +- .../v1/core/super-converter/helpers.test.js | 37 +++++++++++++++++++ .../w/tc/helpers/translate-table-cell.js | 9 +++-- .../w/tc/helpers/translate-table-cell.test.js | 20 +++++++++- 4 files changed, 62 insertions(+), 6 deletions(-) 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 059ccf8e53..e0476c7cd7 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 @@ -530,7 +530,7 @@ const resolveShadingFillColor = (shading) => { if (!shading || typeof shading !== 'object') return null; const fill = normalizeHexColor(shading.fill); - if (!fill) return null; + if (!fill || !isValidHexColor(fill)) return null; const val = typeof shading.val === 'string' ? shading.val.trim().toLowerCase() : ''; const pctMatch = val.match(/^pct(\d{1,3})$/); 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 60ac1a898c..0ef56aff90 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'; describe('polygonToObj', () => { @@ -560,3 +561,39 @@ describe('eighthPointsToPixels', () => { }); }); }); + +describe('resolveShadingFillColor', () => { + it('returns null for null input', () => { + expect(resolveShadingFillColor(null)).toBeNull(); + }); + + it('returns null for non-object input', () => { + expect(resolveShadingFillColor('FFFFFF')).toBeNull(); + }); + + it('returns normalized 6-char hex for a solid fill', () => { + expect(resolveShadingFillColor({ fill: 'FF00FF', val: 'clear' })).toBe('FF00FF'); + }); + + it('normalizes lowercase hex to uppercase', () => { + expect(resolveShadingFillColor({ fill: 'ff00ff' })).toBe('FF00FF'); + }); + + it('returns null for w:fill="auto" (not a valid hex color)', () => { + expect(resolveShadingFillColor({ fill: 'auto', val: 'clear' })).toBeNull(); + }); + + it('returns null when fill is missing', () => { + expect(resolveShadingFillColor({ val: 'clear' })).toBeNull(); + }); + + it('blends foreground into background for pct patterns', () => { + // pct50 = 50% black (#000000) over white (#FFFFFF) → #808080 + const result = resolveShadingFillColor({ fill: 'FFFFFF', color: '000000', val: 'pct50' }); + expect(result).toBe('808080'); + }); + + it('returns the fill directly when val is not a pct pattern', () => { + expect(resolveShadingFillColor({ fill: 'ABCDEF', val: 'solid' })).toBe('ABCDEF'); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js index 9b912fb02f..7679ea1282 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js @@ -1,4 +1,4 @@ -import { pixelsToTwips, inchesToTwips, twipsToPixels } from '@converter/helpers'; +import { pixelsToTwips, inchesToTwips, twipsToPixels, normalizeHexColor, isValidHexColor } from '@converter/helpers'; import { translateChildNodes } from '@converter/v2/exporter/helpers/index'; import { translator as tcPrTranslator } from '../../tcPr'; import { @@ -73,8 +73,11 @@ export function generateTableCellProperties(node) { // Background const { background = {} } = attrs; - if (background?.color && tableCellProperties.shading?.fill !== background?.color) { - tableCellProperties['shading'] = { fill: background.color }; + if (background?.color) { + const fill = normalizeHexColor(background.color); + if (fill && isValidHexColor(fill) && tableCellProperties.shading?.fill?.toUpperCase() !== fill) { + tableCellProperties['shading'] = { fill, val: 'clear' }; + } } else if (!background?.color && tableCellProperties?.shading?.fill) { delete tableCellProperties.shading; } diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.test.js index f8557af901..c16f87823d 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.test.js @@ -43,8 +43,9 @@ describe('translate-table-cell helpers', () => { // gridSpan expect(byName['w:gridSpan'].attributes['w:val']).toBe('2'); - // background - expect(byName['w:shd'].attributes['w:fill']).toBe('#FF00FF'); + // background — OOXML requires bare 6-char hex (no #) and w:val="clear" for solid fills + expect(byName['w:shd'].attributes['w:fill']).toBe('FF00FF'); + expect(byName['w:shd'].attributes['w:val']).toBe('clear'); // tcMar const mar = byName['w:tcMar']; @@ -116,6 +117,21 @@ describe('translate-table-cell helpers', () => { expect(byName['w:tcW']).toBeTruthy(); }); + it('generateTableCellProperties strips # from background color and adds val:clear', () => { + const node = { attrs: { background: { color: '#A1B2C3' } } }; + const tcPr = generateTableCellProperties(node); + const shd = tcPr.elements.find((e) => e.name === 'w:shd'); + expect(shd.attributes['w:fill']).toBe('A1B2C3'); + expect(shd.attributes['w:val']).toBe('clear'); + }); + + it('generateTableCellProperties does not write w:shd for non-hex background (e.g. auto)', () => { + const node = { attrs: { colwidth: [100], widthUnit: 'px', background: { color: 'auto' } } }; + const tcPr = generateTableCellProperties(node); + const shd = tcPr.elements.find((e) => e.name === 'w:shd'); + expect(shd).toBeUndefined(); + }); + it('translateTableCell wraps children with tcPr as the first element', async () => { const params = { node: { attrs: { colwidth: [60], widthUnit: 'px' } },