diff --git a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/index.js b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/index.js index d244450c9f..2584875325 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/index.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/index.js @@ -2,3 +2,4 @@ export * from './docx-helpers.js'; export * from './docx-constants.js'; export * from './get-default-style-definition.js'; +export * from './xml-node-access.js'; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/xml-node-access.js b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/xml-node-access.js new file mode 100644 index 0000000000..e07481f9fd --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/xml-node-access.js @@ -0,0 +1,54 @@ +// @ts-check + +/** + * Safe reads over xml-js element trees. + * + * AIDEV-NOTE: xml-js omits `elements` and `attributes` entirely rather than + * emitting empty ones, so ``, `` and `` all + * parse to `{ type: 'element', name: 'w:rPr' }` with no `elements` key. Every + * child of `CT_Style` is optional in ECMA-376, so empty property containers are + * schema-valid, but readers that assume `node.elements` exists throw on them + * (issue #3861). Reach into parsed OOXML through these helpers rather than + * dereferencing `.elements` or `.attributes` directly. + */ + +/** + * A node in a parsed xml-js tree. + * @typedef {object} XmlNode + * @property {string} [type] Node kind, e.g. `element` or `text`. + * @property {string} [name] Qualified element name, e.g. `w:rPr`. + * @property {Record} [attributes] Absent when the element has no attributes. + * @property {XmlNode[]} [elements] Absent when the element has no children. + * @property {string} [text] Character data, for text nodes. + */ + +/** + * Child elements of a parsed OOXML node, or an empty array when it has none. + * @param {XmlNode | null | undefined} node + * @returns {XmlNode[]} + */ +export const childElements = (node) => (Array.isArray(node?.elements) ? node.elements : []); + +/** + * First child element with the given qualified name. + * @param {XmlNode | null | undefined} node + * @param {string} name Qualified element name, e.g. `w:rPr`. + * @returns {XmlNode | undefined} + */ +export const findChild = (node, name) => childElements(node).find((el) => el?.name === name); + +/** + * Every child element with the given qualified name. + * @param {XmlNode | null | undefined} node + * @param {string} name Qualified element name, e.g. `w:tblStylePr`. + * @returns {XmlNode[]} + */ +export const findChildren = (node, name) => childElements(node).filter((el) => el?.name === name); + +/** + * Attribute value, or undefined when the element carries no attributes. + * @param {XmlNode | null | undefined} node + * @param {string} name Qualified attribute name, e.g. `w:val`. + * @returns {string | undefined} + */ +export const attrValue = (node, name) => node?.attributes?.[name]; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.empty-style-properties.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.empty-style-properties.test.js new file mode 100644 index 0000000000..4f6baca966 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.empty-style-properties.test.js @@ -0,0 +1,84 @@ +// @ts-check +import { describe, expect, it } from 'vitest'; +import * as xmljs from 'xml-js'; +import { createDocumentJson } from './docxImporter.js'; + +/** + * End-to-end import cover for empty OOXML property containers (issue #3861). + * + * AIDEV-NOTE: Fixtures are parsed from XML strings because xml-js omits the + * `elements` key on empty elements. Object literals using `elements: []` do not + * reproduce the failing shape. + */ + +const WORDPROCESSING_NS = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'; +const RELATIONSHIPS_NS = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships'; + +const parse = (xml) => xmljs.xml2js(xml, { compact: false }); + +const DOCUMENT_XML = ` + + + before + + + + cell + + after + + +`; + +/** Styles.xml where Table Grid inherits from a bare base and carries an empty w:rPr. */ +const STYLES_XML = ` + + + + + + + + +`; + +const buildDocx = () => ({ + 'word/document.xml': parse(DOCUMENT_XML), + 'word/styles.xml': parse(STYLES_XML), + 'word/_rels/document.xml.rels': parse( + ``, + ), +}); + +const buildConverter = () => ({ + headerIds: {}, + headers: {}, + footers: {}, + docHiglightColors: new Set(), + trackedChangesOptions: {}, + convertedXml: {}, +}); + +const textOf = (node) => { + if (!node) return ''; + if (node.type === 'text') return node.text ?? ''; + return (node.content ?? []).map(textOf).join(''); +}; + +describe('DOCX import with empty style property containers', () => { + it('keeps a table whose style has an empty w:rPr and a bare basedOn target', () => { + const exceptions = []; + const editor = { + emit: (name, payload) => { + if (name === 'exception') exceptions.push(payload.error); + }, + options: {}, + }; + + const result = createDocumentJson(buildDocx(), buildConverter(), editor); + + const body = (result?.pmDoc?.content ?? []).map((node) => `${node.type}:${textOf(node)}`); + expect(body).toEqual(['paragraph:before', 'table:cell', 'paragraph:after']); + expect(exceptions).toEqual([]); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.empty-style-properties.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.empty-style-properties.test.js new file mode 100644 index 0000000000..416becc526 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.empty-style-properties.test.js @@ -0,0 +1,175 @@ +// @ts-check +import { describe, expect, it } from 'vitest'; +import * as xmljs from 'xml-js'; +import { _getReferencedTableStyles } from './tbl-translator.js'; + +/** + * Empty property containers in `word/styles.xml` used to abort table import (issue #3861). + * + * AIDEV-NOTE: These fixtures parse real XML strings on purpose. xml-js omits the + * `elements` key for empty elements, and hand-authored nodes using `elements: []` + * do NOT reproduce that shape, so object-literal fixtures pass against the bug. + * Keep building these from XML. + */ + +const WORDPROCESSING_NS = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'; + +/** + * Build a `docx` bundle whose Table Grid style carries the given child XML. + * @param {string} tableGridChildren Raw XML placed inside ``. + * @param {string} [extraStyles] Raw XML for sibling `` elements. + */ +const docxWithTableGrid = (tableGridChildren, extraStyles = '') => ({ + 'word/styles.xml': xmljs.xml2js( + ` + + + ${extraStyles} + + + ${tableGridChildren} + + `, + { compact: false }, + ), +}); + +const resolve = (docx, styleId = 'TableGrid') => _getReferencedTableStyles(styleId, { docx }); + +describe('_getReferencedTableStyles with empty style properties', () => { + describe('empty run properties (the reported case)', () => { + it.each([ + ['self-closing', ''], + ['self-closing with space', ''], + ['paired', ''], + ['whitespace only', '\n '], + ])('treats a %s w:rPr as a no-op', (_label, rPr) => { + const styles = resolve(docxWithTableGrid(rPr)); + + expect(styles).not.toBeNull(); + expect(styles?.fonts).toBeUndefined(); + expect(styles?.fontSize).toBeUndefined(); + }); + + it('still reads populated run properties', () => { + const styles = resolve( + docxWithTableGrid(''), + ); + + expect(styles?.fonts).toEqual({ ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial' }); + expect(styles?.fontSize).toBe('12pt'); + }); + + it('treats a w:rFonts without attributes as a no-op', () => { + const styles = resolve(docxWithTableGrid('')); + + expect(styles?.fonts).toBeUndefined(); + }); + }); + + describe('empty paragraph properties', () => { + it.each([ + ['self-closing', ''], + ['paired', ''], + ])('treats a %s w:pPr as a no-op', (_label, pPr) => { + const styles = resolve(docxWithTableGrid(pPr)); + + expect(styles?.justification).toBeUndefined(); + }); + + it('still reads justification from a populated w:pPr', () => { + const styles = resolve(docxWithTableGrid('')); + + expect(styles?.justification).toBe('center'); + }); + }); + + describe('inherited styles', () => { + it('tolerates a w:basedOn target that has no children', () => { + const docx = docxWithTableGrid( + '', + '', + ); + + expect(() => resolve(docx)).not.toThrow(); + }); + + it('tolerates a w:basedOn target that does not exist', () => { + expect(() => resolve(docxWithTableGrid(''))).not.toThrow(); + }); + + it('still inherits table properties from a populated base style', () => { + const docx = docxWithTableGrid( + '', + ` + + `, + ); + + expect(resolve(docx)?.borders?.top).toMatchObject({ val: 'single' }); + }); + }); + + describe('conditional formatting', () => { + it('skips a w:tblStylePr that has no w:type to key it by', () => { + const styles = resolve(docxWithTableGrid('')); + + expect(styles).not.toBeNull(); + expect(Object.keys(styles ?? {})).not.toContain('undefined'); + }); + + it('still reads a typed w:tblStylePr', () => { + const styles = resolve(docxWithTableGrid('')); + + expect(styles?.firstRow).toBeDefined(); + }); + + it('treats empty conditional property containers as no-ops', () => { + const styles = resolve( + docxWithTableGrid(''), + ); + + expect(styles?.firstRow).toBeDefined(); + }); + }); + + describe('malformed style records', () => { + it('tolerates a matched style element with no children', () => { + const docx = { + 'word/styles.xml': xmljs.xml2js( + ` + + `, + { compact: false }, + ), + }; + + expect(() => resolve(docx)).not.toThrow(); + }); + + it('ignores style elements that carry no attributes', () => { + const docx = { + 'word/styles.xml': xmljs.xml2js( + ` + + + `, + { compact: false }, + ), + }; + + expect(resolve(docx)?.name).toBeDefined(); + }); + + it('returns null when styles.xml is absent', () => { + expect(resolve({})).toBeNull(); + }); + + it('treats empty w:tblPr, w:tcPr and w:trPr as no-ops', () => { + const styles = resolve(docxWithTableGrid('')); + + expect(styles?.borders).toBeUndefined(); + expect(styles?.cellMargins).toBeUndefined(); + }); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.js index 2399460cab..70963785c3 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tbl/tbl-translator.js @@ -1,5 +1,11 @@ // @ts-check import { translator as tblStylePrTranslator } from '@converter/v3/handlers/w/tblStylePr'; +import { + attrValue, + childElements, + findChild, + findChildren, +} from '@core/super-converter/docx-helpers/xml-node-access.js'; import { preProcessVerticalMergeCells } from '@core/super-converter/export-helpers/pre-process-vertical-merge-cells.js'; import { eighthPointsToPixels, halfPointToPoints, twipsToPixels } from '@core/super-converter/helpers.js'; import { buildFallbackGridForTable } from '@core/super-converter/helpers/tableFallbackHelpers.js'; @@ -370,44 +376,40 @@ export function _getReferencedTableStyles(tableStyleReference, params) { // Find the style tag in styles.xml const { docx } = params; - const styles = docx['word/styles.xml']; - const { elements } = styles.elements[0]; - const styleElements = elements.filter((el) => el.name === 'w:style'); - const styleTag = styleElements.find((el) => el.attributes['w:styleId'] === tableStyleReference); + const styles = docx?.['word/styles.xml']; + const styleElements = findChildren(childElements(styles)[0], 'w:style'); + const styleTag = styleElements.find((el) => attrValue(el, 'w:styleId') === tableStyleReference); if (!styleTag) return null; - stylesToReturn.name = styleTag.elements.find((el) => el.name === 'w:name'); + stylesToReturn.name = findChild(styleTag, 'w:name'); // Find style it is based on, if any, to inherit table properties from - const basedOn = styleTag.elements.find((el) => el.name === 'w:basedOn'); + const basedOn = findChild(styleTag, 'w:basedOn'); let baseTblPr; if (basedOn?.attributes) { - const baseStyles = styleElements.find((el) => el.attributes['w:styleId'] === basedOn.attributes['w:val']); - baseTblPr = baseStyles ? baseStyles.elements.find((el) => el.name === 'w:tblPr') : {}; + const baseStyles = styleElements.find((el) => attrValue(el, 'w:styleId') === attrValue(basedOn, 'w:val')); + baseTblPr = baseStyles ? findChild(baseStyles, 'w:tblPr') : {}; } // Find paragraph properties to get justification - const pPr = styleTag.elements.find((el) => el.name === 'w:pPr'); - if (pPr) { - const justification = pPr.elements.find((el) => el.name === 'w:jc'); - if (justification?.attributes) stylesToReturn.justification = justification.attributes['w:val']; - } + const justification = findChild(findChild(styleTag, 'w:pPr'), 'w:jc'); + if (justification?.attributes) stylesToReturn.justification = attrValue(justification, 'w:val'); // Find run properties to get fonts and font size - const rPr = styleTag?.elements.find((el) => el.name === 'w:rPr'); + const rPr = findChild(styleTag, 'w:rPr'); if (rPr) { - const fonts = rPr.elements.find((el) => el.name === 'w:rFonts'); - if (fonts) { + const fonts = findChild(rPr, 'w:rFonts'); + if (fonts?.attributes) { const { 'w:ascii': ascii, 'w:hAnsi': hAnsi, 'w:cs': cs } = fonts.attributes; stylesToReturn.fonts = { ascii, hAnsi, cs }; } - const fontSize = rPr.elements.find((el) => el.name === 'w:sz'); - if (fontSize?.attributes) stylesToReturn.fontSize = halfPointToPoints(fontSize.attributes['w:val']) + 'pt'; + const fontSize = findChild(rPr, 'w:sz'); + if (fontSize?.attributes) stylesToReturn.fontSize = halfPointToPoints(attrValue(fontSize, 'w:val')) + 'pt'; } // Find table properties to get borders and cell margins - const tblPr = styleTag.elements.find((el) => el.name === 'w:tblPr'); + const tblPr = findChild(styleTag, 'w:tblPr'); if (tblPr && tblPr.elements) { // Merge base + current for encoding only; do not mutate styles.xml (would duplicate w:tblCellMar etc. per table using this style) const mergedTblPr = @@ -437,14 +439,12 @@ export function _getReferencedTableStyles(tableStyleReference, params) { } } - const tblStylePr = styleTag.elements.filter((el) => el.name === 'w:tblStylePr'); - let styleProps = {}; - if (tblStylePr) { - styleProps = tblStylePr.reduce((acc, el) => { - acc[el.attributes['w:type']] = tblStylePrTranslator.encode({ ...params, nodes: [el] }); - return acc; - }, {}); - } + // Conditional formatting is keyed by w:type; entries without one cannot be addressed by the cascade. + const styleProps = findChildren(styleTag, 'w:tblStylePr').reduce((acc, el) => { + const conditionalType = attrValue(el, 'w:type'); + if (conditionalType) acc[conditionalType] = tblStylePrTranslator.encode({ ...params, nodes: [el] }); + return acc; + }, {}); return { ...stylesToReturn,