Skip to content

Commit 84aa0e4

Browse files
artem-harbourArtem Nistuley
andauthored
fix: prevent corrupt docx export for legacy w:pict vml rect content (#2905)
Co-authored-by: Artem Nistuley <artem@superdoc.dev>
1 parent 0038b2c commit 84aa0e4

6 files changed

Lines changed: 117 additions & 18 deletions

File tree

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/mc/altermateContent/alternate-content-translator.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ function encode(params) {
5757
function decode(params) {
5858
const { node } = params;
5959
const { drawingContent } = node.attrs;
60+
if (!hasValidDrawingContent(drawingContent)) {
61+
return null;
62+
}
6063

6164
// Handle modern DrawingML content (existing logic)
6265
const drawing = {
@@ -70,9 +73,19 @@ function decode(params) {
7073
elements: [drawing],
7174
};
7275

76+
const fallback = {
77+
name: 'mc:Fallback',
78+
elements: [
79+
{
80+
name: 'w:drawing',
81+
elements: carbonCopy(drawing.elements || []),
82+
},
83+
],
84+
};
85+
7386
return {
7487
name: 'mc:AlternateContent',
75-
elements: [choice],
88+
elements: [choice, fallback],
7689
};
7790
}
7891

@@ -139,3 +152,18 @@ function buildPath(existingPath = [], node, branch) {
139152
if (branch) path.push(branch);
140153
return path;
141154
}
155+
156+
/**
157+
* @param {unknown} drawingContent
158+
* @returns {boolean}
159+
*/
160+
function hasValidDrawingContent(drawingContent) {
161+
const drawingChildren = drawingContent?.elements;
162+
if (!Array.isArray(drawingChildren) || drawingChildren.length === 0) {
163+
return false;
164+
}
165+
166+
return drawingChildren.some(
167+
(child) => child && typeof child === 'object' && (child.name === 'wp:inline' || child.name === 'wp:anchor'),
168+
);
169+
}

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/mc/altermateContent/alternate-content-translator.test.js

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ describe('mc:AltermateContent translator', () => {
125125
});
126126

127127
describe('decode', () => {
128-
it('returns mc:AlternateContent structure with w:drawing inside mc:Choice', () => {
128+
it('returns mc:AlternateContent with valid choice and fallback drawing branches', () => {
129129
const params = {
130130
node: {
131131
attrs: {
@@ -149,30 +149,25 @@ describe('mc:AltermateContent translator', () => {
149149
},
150150
],
151151
},
152-
],
153-
});
154-
});
155-
156-
it('handles empty drawingContent gracefully', () => {
157-
const params = { node: { attrs: {} } };
158-
const result = translator.decode(params);
159-
160-
expect(result).toEqual({
161-
name: 'mc:AlternateContent',
162-
elements: [
163152
{
164-
name: 'mc:Choice',
165-
attributes: { Requires: 'wps' },
153+
name: 'mc:Fallback',
166154
elements: [
167155
{
168156
name: 'w:drawing',
169-
elements: [],
157+
elements: [{ name: 'wp:inline' }],
170158
},
171159
],
172160
},
173161
],
174162
});
175163
});
164+
165+
it('returns null when drawingContent is missing/invalid', () => {
166+
const params = { node: { attrs: {} } };
167+
const result = translator.decode(params);
168+
169+
expect(result).toBeNull();
170+
});
176171
});
177172
});
178173

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/pict/helpers/translate-content-block.js

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,43 @@ import { wrapTextInRun } from '@converter/exporter';
88
*/
99
export function translateContentBlock(params) {
1010
const { node } = params;
11-
const { vmlAttributes, horizontalRule } = node.attrs;
11+
const { vmlAttributes, horizontalRule, attributes, style } = node.attrs;
12+
const hasLegacyVmlMarkers = detectLegacyVmlRectMarkers(attributes, style);
1213

1314
// Handle VML v:rect elements (like horizontal rules)
14-
if (vmlAttributes || horizontalRule) {
15+
if (vmlAttributes || horizontalRule || hasLegacyVmlMarkers) {
1516
return translateVRectContentBlock(params);
1617
}
1718

1819
const alternateContent = alternateChoiceTranslator.decode(params);
20+
if (!alternateContent) {
21+
return null;
22+
}
1923
return wrapTextInRun(alternateContent);
2024
}
2125

26+
/**
27+
* Detects legacy VML rect signatures preserved in contentBlock attrs.
28+
* This prevents generic legacy w:pict content from being routed into
29+
* DrawingML alternate-content export paths.
30+
*
31+
* @param {Record<string, unknown>|null|undefined} rawAttributes
32+
* @param {unknown} style
33+
* @returns {boolean}
34+
*/
35+
function detectLegacyVmlRectMarkers(rawAttributes, style) {
36+
if (style) return true;
37+
if (!rawAttributes || typeof rawAttributes !== 'object') return false;
38+
39+
const attrs = rawAttributes;
40+
41+
if (typeof attrs.style === 'string' && attrs.style.trim().length > 0) return true;
42+
if (attrs.fillcolor != null) return true;
43+
if (attrs.stroked != null) return true;
44+
45+
return Object.keys(attrs).some((key) => key.startsWith('o:'));
46+
}
47+
2248
// Nominal full-width value for VML style. Word ignores this when o:hr="t"
2349
// is present and renders the rect at full page width instead.
2450
const FULL_WIDTH_PT = '468pt';

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/pict/helpers/translate-content-block.test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,42 @@ describe('translateContentBlock', () => {
6363
expect(alternateChoiceTranslator.decode).not.toHaveBeenCalled();
6464
expect(result.elements[0].name).toBe('w:pict');
6565
});
66+
67+
it('should use translateVRectContentBlock when legacy VML markers exist in attributes', () => {
68+
const params = {
69+
node: {
70+
attrs: {
71+
attributes: {
72+
style: 'position:absolute;width:100pt;height:20pt',
73+
fillcolor: 'yellow',
74+
stroked: 'f',
75+
},
76+
},
77+
},
78+
};
79+
80+
generateRandomSigned32BitIntStrId.mockReturnValue('12345678');
81+
82+
const result = translateContentBlock(params);
83+
84+
expect(alternateChoiceTranslator.decode).not.toHaveBeenCalled();
85+
expect(result.elements[0].name).toBe('w:pict');
86+
});
87+
88+
it('should return null when alternateChoiceTranslator returns null', () => {
89+
alternateChoiceTranslator.decode.mockReturnValue(null);
90+
91+
const params = {
92+
node: {
93+
attrs: {},
94+
},
95+
};
96+
97+
const result = translateContentBlock(params);
98+
99+
expect(result).toBeNull();
100+
expect(wrapTextInRun).not.toHaveBeenCalled();
101+
});
66102
});
67103

68104
describe('translateVRectContentBlock', () => {

packages/super-editor/src/editors/v1/extensions/content-block/content-block.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ export const ContentBlock = Node.create({
142142
rendered: false,
143143
},
144144

145+
vmlAttributes: {
146+
default: null,
147+
rendered: false,
148+
},
149+
150+
style: {
151+
default: null,
152+
rendered: false,
153+
},
154+
145155
attributes: {
146156
rendered: false,
147157
},

packages/super-editor/src/editors/v1/extensions/types/node-attributes.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,10 @@ export interface ContentBlockAttrs extends InlineNodeAttributes {
986986
size?: ContentBlockSize | null;
987987
/** Background color */
988988
background?: string | null;
989+
/** @internal VML attributes for legacy w:pict round-trip */
990+
vmlAttributes?: Record<string, unknown> | null;
991+
/** @internal Preserved inline style for legacy VML content */
992+
style?: string | null;
989993
/** @internal Drawing content data */
990994
drawingContent?: unknown;
991995
/** @internal Attributes storage */

0 commit comments

Comments
 (0)