Skip to content

fix(super-converter): keep the document when a style record is unreadable (#3861) #3865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
caio-pizzol wants to merge 2 commits into from
Closed

fix(super-converter): keep the document when a style record is unreadable (#3861) #3865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e008935
fix(super-converter): keep the document when a style record is unread…
caio-pizzol Aug 4, 2026
b7f9d2d
fix(super-converter): validate outline levels and tab positions in full
caio-pizzol Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions .../super-converter/docx-helpers/get-default-style-definition.empty-style-properties.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 `<w:style w:styleId="Target">`. */
const parseStyle = (styleChildren) =>
getDefaultStyleDefinition('Target', {
'word/styles.xml': xmljs.xml2js(
`<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<w:styles xmlns:w="${WORDPROCESSING_NS}">
<w:style w:type="paragraph" w:styleId="Target">${styleChildren}</w:style>
</w:styles>`,
{ compact: false },
),
});

describe('getDefaultStyleDefinition with incomplete properties', () => {
describe('w:outlineLvl', () => {
it('reads a valid level', () => {
expect(parseStyle('<w:pPr><w:outlineLvl w:val="2"/></w:pPr>').attrs.outlineLevel).toBe(2);
});

it('reads a negative level', () => {
expect(parseStyle('<w:pPr><w:outlineLvl w:val="-1"/></w:pPr>').attrs.outlineLevel).toBe(-1);
});

it('tolerates surrounding whitespace', () => {
expect(parseStyle('<w:pPr><w:outlineLvl w:val=" 4 "/></w:pPr>').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', '<w:outlineLvl/>'],
['a non-numeric w:val', '<w:outlineLvl w:val="abc"/>'],
['an empty w:val', '<w:outlineLvl w:val=""/>'],
['a numeric prefix', '<w:outlineLvl w:val="2abc"/>'],
['a decimal', '<w:outlineLvl w:val="3.9"/>'],
['exponent notation', '<w:outlineLvl w:val="1e2"/>'],
['hex notation', '<w:outlineLvl w:val="0x10"/>'],
])('reports null for %s', (_label, outlineLvl) => {
expect(parseStyle(`<w:pPr>${outlineLvl}</w:pPr>`).attrs.outlineLevel).toBeNull();
});
});

describe('w:tab', () => {
it('reads a complete tab stop', () => {
expect(parseStyle('<w:pPr><w:tabs><w:tab w:val="left" w:pos="720"/></w:tabs></w:pPr>').styles.tabStops).toEqual([
{ val: 'start', pos: 48, leader: undefined },
]);
});

it('reads a negative position', () => {
expect(parseStyle('<w:pPr><w:tabs><w:tab w:val="left" w:pos="-720"/></w:tabs></w:pPr>').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', '<w:tab/>'],
['only w:val', '<w:tab w:val="left"/>'],
['only w:pos', '<w:tab w:pos="720"/>'],
['an empty w:pos', '<w:tab w:val="left" w:pos=""/>'],
['a whitespace-only w:pos', '<w:tab w:val="left" w:pos=" "/>'],
['a non-numeric w:pos', '<w:tab w:val="left" w:pos="abc"/>'],
])('drops a tab stop with %s', (_label, tab) => {
expect(parseStyle(`<w:pPr><w:tabs>${tab}</w:tabs></w:pPr>`).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('<w:pPr><w:tabs><w:tab w:val="left" w:pos="1.5in"/></w:tabs></w:pPr>').styles.tabStops,
).toBeNull();
});

it('keeps complete stops alongside incomplete ones', () => {
const styles = parseStyle(
'<w:pPr><w:tabs><w:tab w:val="left"/><w:tab w:val="right" w:pos="1440"/></w:tabs></w:pPr>',
).styles;

expect(styles.tabStops).toEqual([{ val: 'end', pos: 96, leader: undefined }]);
});
});

describe('duplicate w:styleId records', () => {
const duplicated = `
<w:style w:type="paragraph" w:styleId="Target"><w:qFormat/></w:style>
<w:style w:type="paragraph" w:styleId="Target">
<w:name w:val="FromSecond"/>
<w:basedOn w:val="BaseFromSecond"/>
<w:pPr><w:jc w:val="center"/><w:ind w:left="720"/></w:pPr>
</w:style>`;

const parsed = () =>
getDefaultStyleDefinition('Target', {
'word/styles.xml': xmljs.xml2js(
`<?xml version="1.0"?><w:styles xmlns:w="${WORDPROCESSING_NS}">${duplicated}</w:styles>`,
{ 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();
});
});
});
115 changes: 64 additions & 51 deletions ...r-editor/src/editors/v1/core/super-converter/docx-helpers/get-default-style-definition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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 },
Expand Down
Loading
Loading