Skip to content

Commit ec5db77

Browse files
authored
fix(super-editor): import <w:temporary/> and lock appearance default contract (SD-3111) (#3295)
* fix(super-editor): import <w:temporary/> and lock appearance default contract (SD-3111) Two narrow gaps, both about the import contract for content-control properties: 1. <w:temporary/> was being dropped on import. Word emits it (ECMA-376 §17.5.2.43), our exporter preserves it on round-trip, the editor's node-attrs typedef carries it, the sdt-info-builder maps it to ContentControlInfo.properties.temporary — but handle-structured-content-node.js never read it from sdtPr. So properties.temporary was always undefined, even for parts where the source XML said true. Fix: new extractTemporary helper applies Word's toggle conventions (empty element = true; w:val "true"/"1" = true; "false"/"0" = false). When the element is absent, the attr is NOT stamped — so consumers can distinguish "absent from source" from "explicit false." 2. The appearance default contract was implicit. ECMA-376 lets the source XML omit <w15:appearance>, and Word's effective default is boundingBox. SuperDoc correctly returns undefined when the source omits the element (resolveAppearance validates against the three spec values). But the public type JSDoc said nothing about this, so consumers building UI on top of appearance could not tell whether undefined meant "data missing" or "Word default applies." Fix: JSDoc on ContentControlProperties.appearance now spells out all four states explicitly, including 'undefined → treat as boundingBox.' Same treatment for temporary's 'undefined → treat as false.' No runtime behavior change — pure contract lock. Tests: - 7 new w:temporary parsing cases (empty toggle, w:val true/1/false/0, absent twice, absent-doesn't-stamp). - Existing info-builder tests already cover the attr-to-properties mapping; they still pass. Verified: - super-editor v3/w/sdt suite: 134/134 - @superdoc/document-api: 1398/1398 Out of scope: - The exporter side already round-trips <w:temporary/> (per ticket description). Not modified. - The appearance default behavior is unchanged — only the JSDoc that documents it. * wip(document-api): tighten temporary JSDoc wording per review (SD-3111) The previous wording 'Word removes the SDT once the user fills it in' paraphrases user-visible behavior but isn't quite the spec's framing. Updated to 'When enabled, Word treats the content control as temporary and may remove the SDT wrapper after the user edits/fills the control.' No runtime change. * fix(super-editor): route extractTemporary through shared ST_OnOff parser (SD-3111) Codex bot review caught a real bug: my hand-rolled ST_OnOff check in extractTemporary only recognized '0' and 'false' as off tokens, but the project's shared ST_OnOff convention (token-sets.ts:27) also accepts 'off'. So <w:temporary w:val="off"/> incorrectly returned true, contradicting the source XML's explicit "not temporary". Replaced with parseStrictStOnOff from the same utils.js other ST_OnOff importers use. Gets: 'true'/'1'/'on' → true, 'false'/'0'/'off' → false, invalid tokens → undefined (with the import-diagnostic side effect for consistency with other ST_OnOff properties). Strict ECMA-376 §22.9.2.7 lists only 1|0|true|false, but the project deliberately accepts on/off for Word-in-the-wild robustness — same convention bold/italic/strike already follow. Three new tests: - <w:temporary w:val="on"/> → true - <w:temporary w:val="off"/> → false (the bug) - invalid token → undefined Verified: - 32/32 in handle-structured-content-node.test.js - 137/137 across the SDT handler suite
1 parent 6e02c1c commit ec5db77

3 files changed

Lines changed: 112 additions & 1 deletion

File tree

packages/document-api/src/content-controls/content-controls.types.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,39 @@ export interface RepeatingSectionControlProperties {
124124
export interface ContentControlProperties {
125125
tag?: string;
126126
alias?: string;
127+
/**
128+
* Visual chrome behavior (`<w15:appearance w15:val="…">`).
129+
*
130+
* Returned verbatim from the imported XML. When the source omits
131+
* the element, this field is `undefined` — NOT silently set to
132+
* `boundingBox`. Word's effective default when the element is
133+
* absent is `boundingBox`, but consumers building UI on top of
134+
* appearance (e.g. deciding whether to draw chrome) must apply
135+
* that default themselves; the API does not fabricate it.
136+
*
137+
* Contract:
138+
* - `'boundingBox'` → explicit; show chrome
139+
* - `'tags'` → explicit; show tag markers
140+
* - `'hidden'` → explicit; render transparently
141+
* - `undefined` → source XML omitted the element; treat as
142+
* Word's effective default (`'boundingBox'`).
143+
*/
127144
appearance?: ContentControlAppearance;
128145
color?: string;
129146
placeholder?: string;
130147
showingPlaceholder?: boolean;
148+
/**
149+
* `<w:temporary/>` toggle (ECMA-376 §17.5.2.43).
150+
*
151+
* When enabled, Word treats the content control as temporary and may
152+
* remove the SDT wrapper after the user edits/fills the control.
153+
*
154+
* Returned verbatim from the imported XML:
155+
* - `true` → element present (`<w:temporary/>` or `w:val="true"`/`"1"`)
156+
* - `false` → element present with `w:val="false"`/`"0"`
157+
* - `undefined` → element absent in source; treat as Word's
158+
* effective default (`false`).
159+
*/
131160
temporary?: boolean;
132161
tabIndex?: number;
133162

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { parseAnnotationMarks } from './handle-annotation-node';
2+
import { parseStrictStOnOff } from '../../../utils.js';
23

34
/**
45
* Detect the semantic control type from sdtPr child elements.
@@ -56,6 +57,24 @@ function extractPlaceholder(sdtPr) {
5657
return docPart?.attributes?.['w:val'] ?? null;
5758
}
5859

60+
/**
61+
* Extract the `<w:temporary/>` toggle from sdtPr (ECMA-376 §17.5.2.43).
62+
*
63+
* Delegates to `parseStrictStOnOff` so token recognition matches the
64+
* project's shared ST_OnOff convention (`true`/`1`/`on` → true;
65+
* `false`/`0`/`off` → false). Returns `undefined` when the element is
66+
* absent or carries an invalid token, preserving the "absent vs explicit
67+
* false" distinction at the Document API surface.
68+
*
69+
* @param {Object|null} sdtPr
70+
* @returns {boolean|undefined}
71+
*/
72+
function extractTemporary(sdtPr) {
73+
const el = sdtPr?.elements?.find((e) => e.name === 'w:temporary');
74+
if (!el) return undefined;
75+
return parseStrictStOnOff(el.attributes?.['w:val'], 'temporary', 'w:temporary');
76+
}
77+
5978
/**
6079
* @param {Object} params
6180
* @returns {Object|null}
@@ -84,9 +103,10 @@ export function handleStructuredContentNode(params) {
84103
// Control type detection from sdtPr children
85104
const controlType = detectControlType(sdtPr);
86105

87-
// Appearance and placeholder
106+
// Appearance, placeholder, and temporary toggle
88107
const appearance = extractAppearance(sdtPr);
89108
const placeholder = extractPlaceholder(sdtPr);
109+
const temporary = extractTemporary(sdtPr);
90110

91111
if (!sdtContent) {
92112
return null;
@@ -117,6 +137,10 @@ export function handleStructuredContentNode(params) {
117137
type: controlType,
118138
appearance,
119139
placeholder,
140+
// `temporary` is only set when the XML carries `<w:temporary/>`;
141+
// omitted attrs stay undefined so consumers can distinguish
142+
// "absent from source" from explicit false.
143+
...(temporary !== undefined ? { temporary } : {}),
120144
sdtPr,
121145
},
122146
};

packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,64 @@ describe('handleStructuredContentNode', () => {
225225
});
226226
});
227227

228+
describe('w:temporary parsing (SD-3111)', () => {
229+
const parseTemporary = (sdtPrElements) => {
230+
const node = createNode(sdtPrElements, [{ name: 'w:r', text: 'content' }]);
231+
const params = { nodes: [node], nodeListHandler: mockNodeListHandler };
232+
parseAnnotationMarks.mockReturnValue({ marks: [] });
233+
return handleStructuredContentNode(params).attrs.temporary;
234+
};
235+
236+
it('reads <w:temporary/> as true (empty toggle)', () => {
237+
expect(parseTemporary([{ name: 'w:temporary' }])).toBe(true);
238+
});
239+
240+
it('reads <w:temporary w:val="true"/> as true', () => {
241+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'true' } }])).toBe(true);
242+
});
243+
244+
it('reads <w:temporary w:val="1"/> as true', () => {
245+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': '1' } }])).toBe(true);
246+
});
247+
248+
it('reads <w:temporary w:val="false"/> as false', () => {
249+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'false' } }])).toBe(false);
250+
});
251+
252+
it('reads <w:temporary w:val="0"/> as false', () => {
253+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': '0' } }])).toBe(false);
254+
});
255+
256+
it('reads <w:temporary w:val="on"/> as true (ST_OnOff alias)', () => {
257+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'on' } }])).toBe(true);
258+
});
259+
260+
it('reads <w:temporary w:val="off"/> as false (ST_OnOff alias)', () => {
261+
// Without going through the shared ST_OnOff set this would
262+
// incorrectly fall through to true. See utils.js parseStrictStOnOff.
263+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'off' } }])).toBe(false);
264+
});
265+
266+
it('returns undefined for invalid w:val tokens (parser rejects unknown tokens)', () => {
267+
expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'banana' } }])).toBeUndefined();
268+
});
269+
270+
it('returns undefined (not false) when <w:temporary> is absent', () => {
271+
// Spec contract: absent in source XML stays undefined so consumers
272+
// can distinguish "Word's effective default" from "explicit false".
273+
expect(parseTemporary([])).toBeUndefined();
274+
expect(parseTemporary([{ name: 'w:tag', attributes: { 'w:val': 'unrelated' } }])).toBeUndefined();
275+
});
276+
277+
it('does not stamp temporary on attrs when absent (preserves "undefined" semantics)', () => {
278+
const node = createNode([], [{ name: 'w:r', text: 'content' }]);
279+
const params = { nodes: [node], nodeListHandler: mockNodeListHandler };
280+
parseAnnotationMarks.mockReturnValue({ marks: [] });
281+
const result = handleStructuredContentNode(params);
282+
expect('temporary' in result.attrs).toBe(false);
283+
});
284+
});
285+
228286
describe('controlType detection', () => {
229287
const detectFrom = (sdtPrElements) => {
230288
const node = createNode(sdtPrElements, [{ name: 'w:r', text: 'content' }]);

0 commit comments

Comments
 (0)