Skip to content

Commit 6731108

Browse files
caio-pizzollucbic
andauthored
feat(image): support relative-path images in browser and DOCX export (#2531)
* feat(image): support relative-path images in browser and DOCX export Add relative URL detection, browser-native display with background registration for DOCX export, and security-hardened sanitization of relative paths. - Add `isRelativeUrl` helper to shared url-validation - Resolve relative paths against `window.location.href` with same-origin enforcement - Register relative images in background without removing from DOM - Add per-editor `pendingRelativeRegistrations` dedup guard - Extract `getOrInitMediaStore` shared helper - Add control character rejection for URL sanitization - Handle missing image dimensions in DOCX export (1 EMU fallback) Co-Authored-By: lucbic <lucbic@users.noreply.github.com> * fix(image): address review findings in relative image registration - Use `derivePreferredFileName` instead of inline split for filename extraction (reuses existing helper) - Sync relative image media to Yjs map for collaboration support (matches `handleNodePath` and `uploadAndInsertImage` patterns) - Mark `handleBrowserPath` as @internal (exported for testing only) --------- Co-authored-by: lucbic <lucbic@users.noreply.github.com>
1 parent 96e3a2c commit 6731108

13 files changed

Lines changed: 766 additions & 31 deletions

File tree

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6164,9 +6164,7 @@ describe('DomPainter', () => {
61646164

61656165
const rtlMeasure: Measure = {
61666166
kind: 'paragraph',
6167-
lines: [
6168-
{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 5, width: 80, ascent: 12, descent: 4, lineHeight: 20 },
6169-
],
6167+
lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 5, width: 80, ascent: 12, descent: 4, lineHeight: 20 }],
61706168
totalHeight: 20,
61716169
};
61726170

@@ -6175,9 +6173,7 @@ describe('DomPainter', () => {
61756173
pages: [
61766174
{
61776175
number: 1,
6178-
fragments: [
6179-
{ kind: 'para', blockId: 'rtl-block', fromLine: 0, toLine: 1, x: 0, y: 0, width: 200 },
6180-
],
6176+
fragments: [{ kind: 'para', blockId: 'rtl-block', fromLine: 0, toLine: 1, x: 0, y: 0, width: 200 }],
61816177
},
61826178
],
61836179
};
@@ -6225,8 +6221,14 @@ describe('DomPainter', () => {
62256221
kind: 'paragraph',
62266222
lines: [
62276223
{
6228-
fromRun: 0, fromChar: 0, toRun: 2, toChar: 4,
6229-
width: 160, ascent: 12, descent: 4, lineHeight: 20,
6224+
fromRun: 0,
6225+
fromChar: 0,
6226+
toRun: 2,
6227+
toChar: 4,
6228+
width: 160,
6229+
ascent: 12,
6230+
descent: 4,
6231+
lineHeight: 20,
62306232
segments: [
62316233
{ runIndex: 0, fromChar: 0, toChar: 5, width: 60 },
62326234
{ runIndex: 1, fromChar: 0, toChar: 0, width: 40, x: 60 },
@@ -6481,10 +6483,10 @@ describe('URL sanitization security', () => {
64816483
expect(sanitizeUrl('#top')).toBe('#top');
64826484
});
64836485

6484-
it('blocks relative URLs', () => {
6485-
expect(sanitizeUrl('/path/to/page')).toBeNull();
6486-
expect(sanitizeUrl('./relative/path')).toBeNull();
6487-
expect(sanitizeUrl('../parent/path')).toBeNull();
6486+
it('resolves relative URLs against page origin', () => {
6487+
expect(sanitizeUrl('/path/to/page')).toBe(`${window.location.origin}/path/to/page`);
6488+
expect(sanitizeUrl('./relative/path')).toBe(`${window.location.origin}/relative/path`);
6489+
expect(sanitizeUrl('../parent/path')).toBe(`${window.location.origin}/parent/path`);
64886490
});
64896491

64906492
it('handles empty and whitespace-only URLs', () => {

packages/super-editor/src/core/Editor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ function getCellContentWidthPx(cellNode: PmNode): number {
112112
*/
113113
interface ImageStorage {
114114
media: Record<string, unknown>;
115+
pendingRelativeRegistrations: Set<string>;
115116
}
116117

117118
/**

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/decode-image-node-helpers.test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ vi.mock('@converter/v3/handlers/w/sdt/helpers/translate-field-annotation.js', ()
2626
prepareTextAnnotation: vi.fn(() => ({ type: 'text', text: 'annotation' })),
2727
}));
2828

29+
vi.mock('@converter/image-dimensions.js', () => ({
30+
readImageDimensionsFromDataUri: vi.fn(() => null),
31+
}));
32+
2933
vi.mock(import('@core/helpers/index.js'), async (importOriginal) => {
3034
const actual = await importOriginal();
3135
return {
@@ -108,6 +112,17 @@ describe('translateImageNode', () => {
108112
expect(result.elements).toEqual(expect.arrayContaining([expect.objectContaining({ name: 'a:graphic' })]));
109113
});
110114

115+
it('should use clamped fallback size (1 EMU) when attrs.size is empty', () => {
116+
baseParams.node.attrs.size = {};
117+
118+
const result = translateImageNode(baseParams);
119+
120+
const extent = result.elements.find((e) => e.name === 'wp:extent').attributes;
121+
// resolveExportSize clamps non-finite values to 1 EMU to prevent corrupt OOXML
122+
expect(extent.cx).toBe(1);
123+
expect(extent.cy).toBe(1);
124+
});
125+
111126
it('should generate a new relationship if rId is presented but relation is missing', () => {
112127
baseParams.node.attrs.rId = 'rId123';
113128
translateImageNode(baseParams);

packages/super-editor/src/extensions/image/image.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export const Image = Node.create({
8585
addStorage() {
8686
return {
8787
media: {},
88+
pendingRelativeRegistrations: new Set(),
8889
};
8990
},
9091

Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,290 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import { flushPromises } from '@vue/test-utils';
3+
4+
// ── Hoisted mock objects (available to vi.mock factories) ─────────────
5+
const { mockDecoSet, mockPluginKeyInstance } = vi.hoisted(() => {
6+
const mockDecoSet = {
7+
add: vi.fn(),
8+
map: vi.fn(),
9+
find: vi.fn(() => []),
10+
remove: vi.fn(),
11+
};
12+
mockDecoSet.add.mockReturnValue(mockDecoSet);
13+
mockDecoSet.map.mockReturnValue(mockDecoSet);
14+
mockDecoSet.remove.mockReturnValue(mockDecoSet);
15+
16+
const mockPluginKeyInstance = {
17+
getState: vi.fn(() => ({ set: mockDecoSet })),
18+
};
19+
20+
return { mockDecoSet, mockPluginKeyInstance };
21+
});
22+
23+
// ── ProseMirror mocks ─────────────────────────────────────────────────
24+
vi.mock('prosemirror-state', () => ({
25+
Plugin: vi.fn(),
26+
PluginKey: vi.fn(() => mockPluginKeyInstance),
27+
}));
28+
29+
vi.mock('prosemirror-view', () => ({
30+
Decoration: {
31+
widget: vi.fn(() => ({ type: 'widget' })),
32+
},
33+
DecorationSet: { empty: mockDecoSet },
34+
}));
35+
36+
vi.mock('prosemirror-transform', () => ({
37+
ReplaceStep: class {},
38+
ReplaceAroundStep: class {},
39+
}));
40+
41+
// ── Image helper mocks ───────────────────────────────────────────────
42+
vi.mock('./handleBase64', () => ({
43+
base64ToFile: vi.fn(() => null),
44+
getBase64FileMeta: vi.fn(() => ({ filename: 'image.png' })),
45+
}));
46+
47+
vi.mock('./handleUrl', () => ({
48+
urlToFile: vi.fn(() => Promise.resolve(null)),
49+
validateUrlAccessibility: vi.fn(() => Promise.resolve(false)),
50+
}));
51+
52+
vi.mock('./startImageUpload', () => ({
53+
checkAndProcessImage: vi.fn(),
54+
uploadAndInsertImage: vi.fn(),
55+
}));
56+
57+
vi.mock('@extensions/image/imageHelpers/startImageUpload.js', () => ({
58+
addImageRelationship: vi.fn(() => 'rId99'),
59+
}));
60+
61+
vi.mock('./fileNameUtils.js', () => ({
62+
buildMediaPath: vi.fn((name) => `word/media/${name}`),
63+
ensureUniqueFileName: vi.fn((name) => name),
64+
}));
65+
66+
// ── Imports (after mocks) ─────────────────────────────────────────────
67+
import { Decoration } from 'prosemirror-view';
68+
import { handleBrowserPath } from './imageRegistrationPlugin.js';
69+
import { urlToFile, validateUrlAccessibility } from './handleUrl';
70+
import { addImageRelationship } from '@extensions/image/imageHelpers/startImageUpload.js';
71+
72+
// ── Helpers ───────────────────────────────────────────────────────────
73+
const createImageNode = (attrs) => ({
74+
type: { name: 'image' },
75+
attrs,
76+
nodeSize: 1,
77+
});
78+
79+
const createTrStub = () => ({
80+
delete: vi.fn(),
81+
setMeta: vi.fn(),
82+
setNodeMarkup: vi.fn(),
83+
mapping: {},
84+
doc: {
85+
nodeAt: vi.fn(() => null),
86+
descendants: vi.fn(),
87+
},
88+
});
89+
90+
const createViewStub = () => {
91+
const tr = createTrStub();
92+
return {
93+
state: {
94+
tr,
95+
doc: tr.doc,
96+
},
97+
dispatch: vi.fn(),
98+
};
99+
};
100+
101+
const createEditorStub = () => ({
102+
storage: { image: { media: {}, pendingRelativeRegistrations: new Set() } },
103+
options: { mode: 'docx' },
104+
});
105+
106+
// ── Tests ─────────────────────────────────────────────────────────────
107+
describe('handleBrowserPath', () => {
108+
let tr, state, view, editor;
109+
110+
beforeEach(() => {
111+
vi.clearAllMocks();
112+
113+
mockDecoSet.add.mockReturnValue(mockDecoSet);
114+
mockDecoSet.map.mockReturnValue(mockDecoSet);
115+
mockDecoSet.remove.mockReturnValue(mockDecoSet);
116+
mockDecoSet.find.mockReturnValue([]);
117+
mockPluginKeyInstance.getState.mockReturnValue({ set: mockDecoSet });
118+
119+
tr = createTrStub();
120+
state = { tr };
121+
view = createViewStub();
122+
editor = createEditorStub();
123+
});
124+
125+
it('returns null for empty foundImages', () => {
126+
expect(handleBrowserPath([], editor, view, state)).toBeNull();
127+
});
128+
129+
it('returns null when only relative images are present (no synchronous doc changes)', () => {
130+
const relativeOnly = [
131+
{ node: createImageNode({ src: './img.png' }), pos: 0, id: {} },
132+
{ node: createImageNode({ src: 'images/photo.png' }), pos: 5, id: {} },
133+
];
134+
135+
const result = handleBrowserPath(relativeOnly, editor, view, state);
136+
137+
// No synchronous transaction — relative images stay in the doc
138+
expect(result).toBeNull();
139+
expect(tr.delete).not.toHaveBeenCalled();
140+
});
141+
142+
it('only creates decorations and deletes non-relative images', () => {
143+
const foundImages = [
144+
{ node: createImageNode({ src: 'https://example.com/img.png' }), pos: 0, id: {} },
145+
{ node: createImageNode({ src: './photos/local.jpg' }), pos: 10, id: {} },
146+
{ node: createImageNode({ src: 'data:image/png;base64,AAA' }), pos: 20, id: {} },
147+
];
148+
149+
const result = handleBrowserPath(foundImages, editor, view, state);
150+
151+
expect(result).not.toBeNull();
152+
// Only the HTTP and data URI images get placeholders and deletions
153+
expect(Decoration.widget).toHaveBeenCalledTimes(2);
154+
expect(tr.delete).toHaveBeenCalledTimes(2);
155+
});
156+
157+
it('deletes non-relative image nodes in descending position order', () => {
158+
const foundImages = [
159+
{ node: createImageNode({ src: 'https://a.com/1.png' }), pos: 5, id: {} },
160+
{ node: createImageNode({ src: 'https://a.com/2.png' }), pos: 15, id: {} },
161+
];
162+
163+
handleBrowserPath(foundImages, editor, view, state);
164+
165+
expect(tr.delete).toHaveBeenCalledTimes(2);
166+
const [firstPos] = tr.delete.mock.calls[0];
167+
const [secondPos] = tr.delete.mock.calls[1];
168+
expect(firstPos).toBeGreaterThan(secondPos);
169+
});
170+
});
171+
172+
describe('registerRelativeImages (via handleBrowserPath)', () => {
173+
let view, editor;
174+
175+
beforeEach(() => {
176+
vi.clearAllMocks();
177+
178+
mockDecoSet.add.mockReturnValue(mockDecoSet);
179+
mockDecoSet.map.mockReturnValue(mockDecoSet);
180+
mockDecoSet.remove.mockReturnValue(mockDecoSet);
181+
mockDecoSet.find.mockReturnValue([]);
182+
mockPluginKeyInstance.getState.mockReturnValue({ set: mockDecoSet });
183+
184+
view = createViewStub();
185+
editor = createEditorStub();
186+
});
187+
188+
it('calls urlToFile for relative URLs without CORS check', async () => {
189+
const foundImages = [{ node: createImageNode({ src: './photos/local.jpg' }), pos: 0, id: {} }];
190+
191+
handleBrowserPath(foundImages, editor, view, { tr: createTrStub() });
192+
await flushPromises();
193+
194+
expect(urlToFile).toHaveBeenCalledWith('./photos/local.jpg', 'local.jpg');
195+
expect(validateUrlAccessibility).not.toHaveBeenCalled();
196+
});
197+
198+
it('extracts filename from nested relative path', async () => {
199+
const foundImages = [{ node: createImageNode({ src: 'assets/img/logo.svg' }), pos: 0, id: {} }];
200+
201+
handleBrowserPath(foundImages, editor, view, { tr: createTrStub() });
202+
await flushPromises();
203+
204+
expect(urlToFile).toHaveBeenCalledWith('assets/img/logo.svg', 'logo.svg');
205+
});
206+
207+
it('stores binary in media store and updates node with rId on success', async () => {
208+
// Mock urlToFile to return a File
209+
const mockFile = new File(['binary'], 'photo.jpg', { type: 'image/jpeg' });
210+
urlToFile.mockResolvedValueOnce(mockFile);
211+
212+
// Mock the view so descendants finds the node and nodeAt returns it
213+
const imageNode = createImageNode({ src: './photo.jpg' });
214+
view.state.doc.descendants.mockImplementation((cb) => {
215+
cb(imageNode, 5);
216+
});
217+
view.state.doc.nodeAt.mockReturnValue(imageNode);
218+
view.state.tr.doc.nodeAt = vi.fn(() => imageNode);
219+
220+
const foundImages = [{ node: imageNode, pos: 5, id: {} }];
221+
222+
handleBrowserPath(foundImages, editor, view, { tr: createTrStub() });
223+
224+
// Wait for the full async chain (urlToFile → FileReader → store → dispatch)
225+
await vi.waitFor(() => {
226+
expect(Object.keys(editor.storage.image.media)).toHaveLength(1);
227+
});
228+
229+
// Binary stored in media store
230+
expect(Object.keys(editor.storage.image.media)[0]).toBe('word/media/photo.jpg');
231+
232+
// Relationship created
233+
expect(addImageRelationship).toHaveBeenCalledWith({ editor, path: 'media/photo.jpg' });
234+
235+
// Node updated with rId and originalSrc for export (not deleted)
236+
expect(view.state.tr.setNodeMarkup).toHaveBeenCalledWith(5, undefined, {
237+
...imageNode.attrs,
238+
rId: 'rId99',
239+
originalSrc: 'word/media/photo.jpg',
240+
});
241+
expect(view.dispatch).toHaveBeenCalled();
242+
});
243+
244+
it('skips duplicate relative URL when the first is still being registered', async () => {
245+
// Create a deferred promise so the first urlToFile call hangs
246+
let resolveFirst;
247+
const firstCall = new Promise((r) => (resolveFirst = r));
248+
urlToFile.mockReturnValueOnce(firstCall);
249+
250+
const imageA = { node: createImageNode({ src: 'assets/dup.png' }), pos: 0, id: {} };
251+
const imageB = { node: createImageNode({ src: 'assets/dup.png' }), pos: 5, id: {} };
252+
253+
// Both arrive in the same batch
254+
handleBrowserPath([imageA, imageB], editor, view, { tr: createTrStub() });
255+
await flushPromises();
256+
257+
// Only one fetch is initiated — the second is blocked by pendingRelativeRegistrations
258+
expect(urlToFile).toHaveBeenCalledTimes(1);
259+
260+
// Complete the first registration
261+
resolveFirst(null);
262+
await flushPromises();
263+
264+
// pendingRelativeRegistrations is cleaned up so a future insert could register again
265+
expect(editor.storage.image.pendingRelativeRegistrations.size).toBe(0);
266+
});
267+
268+
it('does not register or dispatch when urlToFile returns null (fetch failure)', async () => {
269+
urlToFile.mockResolvedValueOnce(null);
270+
271+
const foundImages = [{ node: createImageNode({ src: 'assets/missing.png' }), pos: 0, id: {} }];
272+
273+
handleBrowserPath(foundImages, editor, view, { tr: createTrStub() });
274+
await flushPromises();
275+
276+
expect(urlToFile).toHaveBeenCalledWith('assets/missing.png', 'missing.png');
277+
278+
// No media stored
279+
expect(Object.keys(editor.storage.image.media)).toHaveLength(0);
280+
281+
// No relationship created
282+
expect(addImageRelationship).not.toHaveBeenCalled();
283+
284+
// No transaction dispatched
285+
expect(view.dispatch).not.toHaveBeenCalled();
286+
287+
// Pending set cleaned up so the src can be retried later
288+
expect(editor.storage.image.pendingRelativeRegistrations.has('assets/missing.png')).toBe(false);
289+
});
290+
});

0 commit comments

Comments
 (0)