Skip to content

Commit ccbb245

Browse files
committed
fix: use preserveOrder parser for correct inline element extraction
The preserveOrder:false XMLParser merges all same-named elements into a single property and concatenates text nodes, losing the position of inline elements like <x id="PH"/> relative to surrounding text. This caused placeholders to appear at wrong positions in extracted source. Switch to a preserveOrder:true ordered parser for source/target extraction, which correctly preserves element ordering. The existing {{marker}} + XliffPlaceholder metadata approach is kept — only the parser that feeds it is fixed.
1 parent dee344c commit ccbb245

5 files changed

Lines changed: 485 additions & 182 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
3+
<file source-language="en" target-language="de" datatype="plaintext" original="messages">
4+
<body>
5+
<trans-unit id="simple_ph">
6+
<source>Click <x id="PH"/> to continue</source>
7+
<target></target>
8+
<note>Simple placeholder test</note>
9+
</trans-unit>
10+
<trans-unit id="interpolation">
11+
<source>Hello <x id="INTERPOLATION"/>, welcome!</source>
12+
<target></target>
13+
</trans-unit>
14+
<trans-unit id="multiple_ph">
15+
<source><x id="START_TAG_SPAN"/>Click here<x id="CLOSE_TAG_SPAN"/> or <x id="START_TAG_SPAN"/>there<x id="CLOSE_TAG_SPAN"/></source>
16+
<target></target>
17+
</trans-unit>
18+
<trans-unit id="no_placeholders">
19+
<source>Simple text without placeholders</source>
20+
<target></target>
21+
</trans-unit>
22+
<trans-unit id="with_existing_target">
23+
<source>Update <x id="PH"/> now</source>
24+
<target>Aktualisiere <x id="PH"/> jetzt</target>
25+
</trans-unit>
26+
</body>
27+
</file>
28+
</xliff>

‎__tests__/unit/extractors/xliff.test.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,90 @@ describe('XliffExtractor', () => {
9595
});
9696
});
9797

98+
describe('extract XLIFF 1.2 with inline elements', () => {
99+
it('should extract <x> inline elements as {{marker}} placeholders', () => {
100+
const fixturePath = path.join(
101+
__dirname,
102+
'../../fixtures/xliff-1.2/messages-with-placeholders.xliff'
103+
);
104+
const content = fs.readFileSync(fixturePath, 'utf-8');
105+
106+
const result = extractor.extract(fixturePath, content, 'de');
107+
108+
const simplePh = result.units.find(u => u.id === 'simple_ph');
109+
expect(simplePh).toBeDefined();
110+
expect(simplePh?.source).toBe('Click {{PH}} to continue');
111+
expect(simplePh?.metadata.placeholders).toBeDefined();
112+
expect(simplePh?.metadata.placeholders).toHaveLength(1);
113+
expect(simplePh?.metadata.placeholders![0].marker).toBe('{{PH}}');
114+
expect(simplePh?.metadata.placeholders![0].tagName).toBe('x');
115+
expect(simplePh?.metadata.placeholders![0].attributes.id).toBe('PH');
116+
});
117+
118+
it('should extract INTERPOLATION placeholder with metadata', () => {
119+
const fixturePath = path.join(
120+
__dirname,
121+
'../../fixtures/xliff-1.2/messages-with-placeholders.xliff'
122+
);
123+
const content = fs.readFileSync(fixturePath, 'utf-8');
124+
125+
const result = extractor.extract(fixturePath, content, 'de');
126+
127+
const interpUnit = result.units.find(u => u.id === 'interpolation');
128+
expect(interpUnit).toBeDefined();
129+
expect(interpUnit?.source).toBe('Hello {{INTERPOLATION}}, welcome!');
130+
expect(interpUnit?.metadata.placeholders).toHaveLength(1);
131+
expect(interpUnit?.metadata.placeholders![0].tagName).toBe('x');
132+
});
133+
134+
it('should extract multiple inline elements as separate placeholders', () => {
135+
const fixturePath = path.join(
136+
__dirname,
137+
'../../fixtures/xliff-1.2/messages-with-placeholders.xliff'
138+
);
139+
const content = fs.readFileSync(fixturePath, 'utf-8');
140+
141+
const result = extractor.extract(fixturePath, content, 'de');
142+
143+
const multiUnit = result.units.find(u => u.id === 'multiple_ph');
144+
expect(multiUnit).toBeDefined();
145+
expect(multiUnit?.source).toContain('{{START_TAG_SPAN}}');
146+
expect(multiUnit?.source).toContain('{{CLOSE_TAG_SPAN}}');
147+
expect(multiUnit?.metadata.placeholders).toBeDefined();
148+
expect(multiUnit?.metadata.placeholders!.length).toBe(4);
149+
});
150+
151+
it('should handle units without inline elements normally', () => {
152+
const fixturePath = path.join(
153+
__dirname,
154+
'../../fixtures/xliff-1.2/messages-with-placeholders.xliff'
155+
);
156+
const content = fs.readFileSync(fixturePath, 'utf-8');
157+
158+
const result = extractor.extract(fixturePath, content, 'de');
159+
160+
const plainUnit = result.units.find(u => u.id === 'no_placeholders');
161+
expect(plainUnit).toBeDefined();
162+
expect(plainUnit?.source).toBe('Simple text without placeholders');
163+
expect(plainUnit?.metadata.placeholders).toBeUndefined();
164+
});
165+
166+
it('should extract placeholders from existing targets', () => {
167+
const fixturePath = path.join(
168+
__dirname,
169+
'../../fixtures/xliff-1.2/messages-with-placeholders.xliff'
170+
);
171+
const content = fs.readFileSync(fixturePath, 'utf-8');
172+
173+
const result = extractor.extract(fixturePath, content, 'de');
174+
175+
const targetUnit = result.units.find(u => u.id === 'with_existing_target');
176+
expect(targetUnit).toBeDefined();
177+
expect(targetUnit?.target).toContain('{{PH}}');
178+
expect(targetUnit?.source).toBe('Update {{PH}} now');
179+
});
180+
});
181+
98182
describe('supported formats', () => {
99183
it('should support xliff-1.2 format', () => {
100184
expect(extractor.supportsFormat('xliff-1.2')).toBe(true);

‎dist/extractors/xliff.d.ts‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ export declare class XliffExtractor extends BaseExtractor {
77
readonly supportedFormats: FormatInfo['format'][];
88
readonly fileExtensions: string[];
99
private parser;
10+
/**
11+
* Ordered parser preserves element position relative to text nodes.
12+
* Required for correct extraction of inline elements like <x id="PH"/>
13+
* interspersed with text content.
14+
*/
15+
private orderedParser;
1016
/**
1117
* Detect XLIFF version from content
1218
*/
@@ -38,26 +44,31 @@ export declare class XliffExtractor extends BaseExtractor {
3844
/** XLIFF inline element tag names that should be treated as placeholders */
3945
private static readonly PLACEHOLDER_TAGS;
4046
/**
41-
* Context object for extracting text with placeholders
47+
* Extract text content from element (plain text, no placeholder tracking).
48+
* Used for notes, context, and other non-inline-element fields.
49+
*/
50+
private extractTextContent;
51+
/**
52+
* Build a map from unit ID to ordered (preserveOrder: true) children of source/target.
53+
* This preserves the position of inline XML elements relative to text nodes.
4254
*/
43-
private createPlaceholderContext;
55+
private buildOrderedSourceMap;
4456
/**
45-
* Extract text content from element, preserving placeholders as markers
46-
* Returns the text with placeholder markers (e.g., {{PH}}, {{0}})
57+
* Get an attribute from a preserveOrder node's ':@' entry.
4758
*/
48-
private extractTextContent;
59+
private getOrderedAttr;
4960
/**
50-
* Extract text and placeholders from element
51-
* Returns text with markers, and populates the placeholders array
61+
* Find the children array of a named element within a preserveOrder children array.
5262
*/
53-
private extractTextWithPlaceholders;
63+
private getOrderedElementChildren;
5464
/**
55-
* Extract a placeholder element and create its marker
65+
* Walk ordered (preserveOrder: true) children and extract text with {{marker}} placeholders.
66+
* Correctly preserves the position of inline elements relative to text.
5667
*/
57-
private extractPlaceholder;
68+
private extractOrderedContent;
5869
/**
59-
* Extract text content and placeholders from a source/target element
60-
* Returns both the text (with placeholder markers) and the placeholder array
70+
* Extract text content and placeholders from ordered children.
71+
* Returns both the text (with placeholder markers) and the placeholder array.
6172
*/
6273
extractContentWithPlaceholders(element: unknown): {
6374
text: string;

0 commit comments

Comments
 (0)