diff --git a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.empty-style-properties.test.js b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.empty-style-properties.test.js new file mode 100644 index 0000000000..68928a63c3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.empty-style-properties.test.js @@ -0,0 +1,130 @@ +// @ts-check +import { describe, expect, it } from 'vitest'; +import * as xmljs from 'xml-js'; +import { getDefaultStyleDefinition } from './get-default-style-definition.js'; + +/** + * Incomplete leaf properties in `word/styles.xml` (issue #3861). + * + * AIDEV-NOTE: Asserts the parsed values, not just that parsing survived. An + * earlier pass returned NaN for outlineLevel and half-formed tab stops, both of + * which a survival-only test would have missed. Fixtures parse real XML because + * xml-js omits the `elements` key on empty elements. + */ + +const WORDPROCESSING_NS = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'; + +/** @param {string} styleChildren Raw XML placed inside ``. */ +const parseStyle = (styleChildren) => + getDefaultStyleDefinition('Target', { + 'word/styles.xml': xmljs.xml2js( + ` + + ${styleChildren} + `, + { compact: false }, + ), + }); + +describe('getDefaultStyleDefinition with incomplete properties', () => { + describe('w:outlineLvl', () => { + it('reads a valid level', () => { + expect(parseStyle('').attrs.outlineLevel).toBe(2); + }); + + it('reads a negative level', () => { + expect(parseStyle('').attrs.outlineLevel).toBe(-1); + }); + + it('tolerates surrounding whitespace', () => { + expect(parseStyle('').attrs.outlineLevel).toBe(4); + }); + + // w:val is ST_DecimalNumber (xsd:int). parseInt reads a numeric prefix, so partly + // numeric values used to import as a different, valid-looking level. + it.each([ + ['no w:val', ''], + ['a non-numeric w:val', ''], + ['an empty w:val', ''], + ['a numeric prefix', ''], + ['a decimal', ''], + ['exponent notation', ''], + ['hex notation', ''], + ])('reports null for %s', (_label, outlineLvl) => { + expect(parseStyle(`${outlineLvl}`).attrs.outlineLevel).toBeNull(); + }); + }); + + describe('w:tab', () => { + it('reads a complete tab stop', () => { + expect(parseStyle('').styles.tabStops).toEqual([ + { val: 'start', pos: 48, leader: undefined }, + ]); + }); + + it('reads a negative position', () => { + expect(parseStyle('').styles.tabStops).toEqual([ + { val: 'start', pos: -48, leader: undefined }, + ]); + }); + + // w:val and w:pos are both required on CT_TabStop. An empty w:pos is the sharp + // case: it converts to a plausible-looking 0 rather than an obvious NaN. + it.each([ + ['no attributes', ''], + ['only w:val', ''], + ['only w:pos', ''], + ['an empty w:pos', ''], + ['a whitespace-only w:pos', ''], + ['a non-numeric w:pos', ''], + ])('drops a tab stop with %s', (_label, tab) => { + expect(parseStyle(`${tab}`).styles.tabStops).toBeNull(); + }); + + // AIDEV-NOTE: w:pos is ST_SignedTwipsMeasure, a union of xsd:integer and + // ST_UniversalMeasure, so "1.5in" is legal OOXML that twipsToPixels cannot yet + // convert. It is dropped rather than emitted with an undefined position. Supporting + // universal measures is a separate gap, not a licence to validate w:pos as an integer. + it('drops a universal-measure position it cannot convert', () => { + expect( + parseStyle('').styles.tabStops, + ).toBeNull(); + }); + + it('keeps complete stops alongside incomplete ones', () => { + const styles = parseStyle( + '', + ).styles; + + expect(styles.tabStops).toEqual([{ val: 'end', pos: 96, leader: undefined }]); + }); + }); + + describe('duplicate w:styleId records', () => { + const duplicated = ` + + + + + + `; + + const parsed = () => + getDefaultStyleDefinition('Target', { + 'word/styles.xml': xmljs.xml2js( + `${duplicated}`, + { compact: false }, + ), + }); + + it('takes identity properties from whichever record declares them', () => { + expect(parsed().attrs).toMatchObject({ name: 'FromSecond', basedOn: 'BaseFromSecond', qFormat: true }); + }); + + // Documents the current boundary: only the identity children above look past + // the first record. Paragraph properties are not merged across duplicates. + it('still reads paragraph properties from the first record only', () => { + expect(parsed().styles.textAlign).toBeUndefined(); + }); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.js b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.js index ea20bc5e90..625015f9f7 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.js @@ -1,6 +1,20 @@ import { parseMarks } from '@converter/v2/importer/index.js'; import { twipsToLines, twipsToPixels } from '@converter/helpers.js'; import { kebabCase } from '@superdoc/common'; +import { attrValue, childElements, findChild, findChildren } from './xml-node-access.js'; + +/** + * First child with the given name across every record sharing a styleId. + * + * Applies only to the identity children read below (w:name, w:basedOn, + * w:qFormat). Paragraph and run properties still come from the first matching + * record alone; this does not merge duplicate style records. + * + * @param {import('./xml-node-access.js').XmlNode[]} records Style elements sharing one w:styleId. + * @param {string} name Qualified child name, e.g. `w:basedOn`. + * @returns {import('./xml-node-access.js').XmlNode | undefined} + */ +const findInAnyRecord = (records, name) => records.map((record) => findChild(record, name)).find(Boolean); /** * Gets the default style definition. @@ -15,32 +29,24 @@ export const getDefaultStyleDefinition = (defaultStyleId, docx) => { const styles = docx['word/styles.xml']; if (!styles) return result; - const { elements } = styles.elements[0]; - const elementsWithId = elements.filter((el) => { - const { attributes } = el; - return attributes && attributes['w:styleId'] === defaultStyleId; - }); + const elementsWithId = childElements(childElements(styles)[0]).filter( + (el) => attrValue(el, 'w:styleId') === defaultStyleId, + ); const firstMatch = elementsWithId[0]; if (!firstMatch) return result; if (!firstMatch.elements) return result; - const qFormat = elementsWithId.find((el) => { - const qFormat = el.elements.find((innerEl) => innerEl.name === 'w:qFormat'); - return qFormat; - }); - - const name = elementsWithId - .find((el) => el.elements.some((inner) => inner.name === 'w:name')) - ?.elements.find((inner) => inner.name === 'w:name')?.attributes['w:val']; + const qFormat = findInAnyRecord(elementsWithId, 'w:qFormat'); + const name = attrValue(findInAnyRecord(elementsWithId, 'w:name'), 'w:val'); // pPr - const pPr = firstMatch.elements.find((el) => el.name === 'w:pPr'); - const spacing = pPr?.elements?.find((el) => el.name === 'w:spacing'); - const justify = pPr?.elements?.find((el) => el.name === 'w:jc'); - const indent = pPr?.elements?.find((el) => el.name === 'w:ind'); - const tabs = pPr?.elements?.find((el) => el.name === 'w:tabs'); + const pPr = findChild(firstMatch, 'w:pPr'); + const spacing = findChild(pPr, 'w:spacing'); + const justify = findChild(pPr, 'w:jc'); + const indent = findChild(pPr, 'w:ind'); + const tabs = findChild(pPr, 'w:tabs'); let lineSpaceBefore, lineSpaceAfter, line; if (spacing?.attributes) { @@ -57,56 +63,63 @@ export const getDefaultStyleDefinition = (defaultStyleId, docx) => { firstLine = twipsToPixels(indent.attributes['w:firstLine']); } - let tabStops = []; - if (tabs) { - tabStops = (tabs.elements || []) - .filter((el) => el.name === 'w:tab') - .map((tab) => { - let val = tab.attributes['w:val']; - if (val == 'left') { - val = 'start'; - } else if (val == 'right') { - val = 'end'; - } - return { - val, - pos: twipsToPixels(tab.attributes['w:pos']), - leader: tab.attributes['w:leader'], - }; - }); - } - - const keepNext = pPr?.elements?.find((el) => el.name === 'w:keepNext'); - const keepLines = pPr?.elements?.find((el) => el.name === 'w:keepLines'); - - const outlineLevel = pPr?.elements?.find((el) => el.name === 'w:outlineLvl'); - const outlineLvlValue = outlineLevel?.attributes['w:val']; - - const pageBreakBefore = pPr?.elements?.find((el) => el.name === 'w:pageBreakBefore'); + // ECMA-376 marks w:val and w:pos required on w:tab (CT_TabStop). w:pos is + // ST_SignedTwipsMeasure, a union of xsd:integer and ST_UniversalMeasure (ยง17.18.81), + // so it is judged by whether it converts to a finite position rather than by matching + // the raw string, which would reject legal values like "1.5in". A record that cannot + // place a stop is dropped rather than emitted half-formed: an empty w:pos otherwise + // converts to a plausible-looking 0. + const tabStops = findChildren(tabs, 'w:tab') + .map((tab) => { + let val = attrValue(tab, 'w:val'); + if (val == 'left') { + val = 'start'; + } else if (val == 'right') { + val = 'end'; + } + const rawPos = attrValue(tab, 'w:pos')?.trim(); + return { + val, + pos: rawPos ? twipsToPixels(rawPos) : undefined, + leader: attrValue(tab, 'w:leader'), + }; + }) + .filter((stop) => stop.val != null && Number.isFinite(stop.pos)); + + const keepNext = findChild(pPr, 'w:keepNext'); + const keepLines = findChild(pPr, 'w:keepLines'); + + // w:val is required on w:outlineLvl and is ST_DecimalNumber (xsd:int). parseInt reads + // a numeric prefix, so "2abc" and "3.9" would import as levels 2 and 3 and silently + // reshape headings and the TOC. The whole value must be an integer or there is no + // level to report. + const outlineLevel = findChild(pPr, 'w:outlineLvl'); + const outlineLvlRaw = attrValue(outlineLevel, 'w:val')?.trim(); + const outlineLvlValue = /^[+-]?\d+$/.test(outlineLvlRaw ?? '') ? Number(outlineLvlRaw) : NaN; + + const pageBreakBefore = findChild(pPr, 'w:pageBreakBefore'); let pageBreakBeforeVal = 0; if (pageBreakBefore) { if (!pageBreakBefore.attributes?.['w:val']) pageBreakBeforeVal = 1; else pageBreakBeforeVal = Number(pageBreakBefore?.attributes?.['w:val']); } - const pageBreakAfter = pPr?.elements?.find((el) => el.name === 'w:pageBreakAfter'); + const pageBreakAfter = findChild(pPr, 'w:pageBreakAfter'); let pageBreakAfterVal; if (pageBreakAfter) { if (!pageBreakAfter.attributes?.['w:val']) pageBreakAfterVal = 1; else pageBreakAfterVal = Number(pageBreakAfter?.attributes?.['w:val']); } - const basedOn = elementsWithId - .find((el) => el.elements.some((inner) => inner.name === 'w:basedOn')) - ?.elements.find((inner) => inner.name === 'w:basedOn')?.attributes['w:val']; + const basedOn = attrValue(findInAnyRecord(elementsWithId, 'w:basedOn'), 'w:val'); - const linkToCharacterStyle = firstMatch.elements.find((el) => el.name === 'w:link')?.attributes?.['w:val'] ?? null; + const linkToCharacterStyle = attrValue(findChild(firstMatch, 'w:link'), 'w:val') ?? null; const parsedAttrs = { name, qFormat: qFormat ? true : false, keepNext: keepNext ? true : false, keepLines: keepLines ? true : false, - outlineLevel: outlineLevel ? parseInt(outlineLvlValue) : null, + outlineLevel: Number.isInteger(outlineLvlValue) ? outlineLvlValue : null, pageBreakBefore: pageBreakBeforeVal ? true : false, pageBreakAfter: pageBreakAfterVal ? true : false, basedOn: basedOn ?? null, @@ -115,7 +128,7 @@ export const getDefaultStyleDefinition = (defaultStyleId, docx) => { }; // rPr - const rPr = firstMatch.elements.find((el) => el.name === 'w:rPr'); + const rPr = findChild(firstMatch, 'w:rPr'); const parsedMarks = parseMarks(rPr, [], docx) || []; const parsedStyles = { spacing: { lineSpaceAfter, lineSpaceBefore, line }, 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..e7652342e0 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.empty-style-properties.test.js @@ -0,0 +1,140 @@ +// @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 + + +`; + +/** Table Grid inherits from a bare base and carries an empty w:rPr. */ +const TABLE_STYLES = ` + + + + + + `; + +/** + * @param {string} [extraStyles] Raw XML for additional `` elements. + */ +const buildDocx = (extraStyles = '') => ({ + 'word/document.xml': parse(DOCUMENT_XML), + 'word/styles.xml': parse( + ` + + + ${TABLE_STYLES} + ${extraStyles} + `, + ), + '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(''); +}; + +/** + * Import a document whose body is always `before` / table / `after`. + * @param {string} [extraStyles] Raw XML for additional `` elements. + */ +const importDocument = (extraStyles) => { + const exceptions = []; + const editor = { + emit: (name, payload) => { + if (name === 'exception') exceptions.push(payload.error); + }, + options: {}, + }; + + const result = createDocumentJson(buildDocx(extraStyles), buildConverter(), editor); + + return { + body: (result?.pmDoc?.content ?? []).map((node) => `${node.type}:${textOf(node)}`), + styleIds: (result?.linkedStyles ?? []).map((style) => style.id), + exceptions, + }; +}; + +const FULL_BODY = ['paragraph:before', 'table:cell', 'paragraph:after']; + +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 { body, exceptions } = importDocument(); + + expect(body).toEqual(FULL_BODY); + expect(exceptions).toEqual([]); + }); + + // AIDEV-NOTE: The style catalogue is built outside the importer's per-node recovery + // boundary, so one unreadable w:style used to abort createDocumentJson entirely. + // getSchema then returned null and createDocument fell back to an empty document, + // losing the whole body rather than one node. These cases must stay import-level. + describe('incomplete style records do not discard the document', () => { + it.each([ + [ + 'w:outlineLvl without w:val', + '', + ], + [ + 'w:tab without attributes', + '', + ], + [ + 'a duplicate w:styleId whose second record is empty', + '', + ], + ['a w:style without attributes', ''], + ])('imports the full body when styles.xml contains %s', (_label, extraStyles) => { + const { body, exceptions } = importDocument(extraStyles); + + expect(body).toEqual(FULL_BODY); + expect(exceptions).toEqual([]); + }); + + it('omits unusable style records from the catalogue but keeps the rest', () => { + const { styleIds } = importDocument(''); + + expect(styleIds).toContain('TableGrid'); + expect(styleIds).not.toContain(undefined); + }); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js index 9cbb01d27d..237255020e 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v2/importer/docxImporter.js @@ -23,7 +23,7 @@ import { pictNodeHandlerEntity } from './pictNodeImporter.js'; import { importCommentData } from './documentCommentsImporter.js'; import { buildTrackedChangeIdMap, buildTrackedChangeIdMapsByPart } from './trackedChangeIdMapper.js'; import { importFootnoteData, importEndnoteData } from './documentFootnotesImporter.js'; -import { getDefaultStyleDefinition } from '@converter/docx-helpers/index.js'; +import { attrValue, getDefaultStyleDefinition } from '@converter/docx-helpers/index.js'; import { pruneIgnoredNodes } from './ignoredNodes.js'; import { tabNodeEntityHandler } from './tabImporter.js'; import { noBreakHyphenNodeEntityHandler } from './noBreakHyphenImporter.js'; @@ -725,34 +725,15 @@ function getStyleDefinitions(docx) { if (!styles) return []; const elements = styles.elements?.[0]?.elements ?? []; - const styleDefinitions = elements.filter((el) => el.name === 'w:style'); - - // Track latent style exceptions - const latentStyles = elements.find((el) => el.name === 'w:latentStyles'); - const matchedLatentStyles = []; - (latentStyles?.elements ?? []).forEach((el) => { - const { attributes } = el; - const match = styleDefinitions.find((style) => style.attributes['w:styleId'] === attributes['w:name']); - if (match) matchedLatentStyles.push(el); - }); - - // Parse all styles - const allParsedStyles = []; - styleDefinitions.forEach((style) => { - const id = style.attributes['w:styleId']; - const parsedStyle = getDefaultStyleDefinition(id, docx); - - const importedStyle = { - id: style.attributes['w:styleId'], - type: style.attributes['w:type'], - definition: parsedStyle, - attributes: {}, - }; + // A w:style without w:styleId cannot be referenced by the document, so it is not a usable definition. + const styleDefinitions = elements.filter((el) => el.name === 'w:style' && attrValue(el, 'w:styleId')); - allParsedStyles.push(importedStyle); - }); - - return allParsedStyles; + return styleDefinitions.map((style) => ({ + id: attrValue(style, 'w:styleId'), + type: attrValue(style, 'w:type'), + definition: getDefaultStyleDefinition(attrValue(style, 'w:styleId'), docx), + attributes: {}, + })); } export function translateStyleDefinitions(docx) { 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,