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/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
index 4f6baca966..e7652342e0 100644
--- 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
@@ -30,21 +30,28 @@ const DOCUMENT_XML = `
`;
-/** Styles.xml where Table Grid inherits from a bare base and carries an empty w:rPr. */
-const STYLES_XML = `
-
-
+/** Table Grid inherits from a bare base and carries an empty w:rPr. */
+const TABLE_STYLES = `
-
-`;
+ `;
-const buildDocx = () => ({
+/**
+ * @param {string} [extraStyles] Raw XML for additional `` elements.
+ */
+const buildDocx = (extraStyles = '') => ({
'word/document.xml': parse(DOCUMENT_XML),
- 'word/styles.xml': parse(STYLES_XML),
+ 'word/styles.xml': parse(
+ `
+
+
+ ${TABLE_STYLES}
+ ${extraStyles}
+ `,
+ ),
'word/_rels/document.xml.rels': parse(
``,
),
@@ -65,20 +72,69 @@ const textOf = (node) => {
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 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']);
+ 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) {