From 5218b52d8246aa4cebe974b6a49b5ade62f590cd Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 10:02:23 -0300 Subject: [PATCH 1/3] fix(super-editor): import and lock appearance default contract (SD-3111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two narrow gaps, both about the import contract for content-control properties: 1. 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 , 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 (per ticket description). Not modified. - The appearance default behavior is unchanged — only the JSDoc that documents it. --- .../content-controls.types.ts | 27 ++++++++++++ .../helpers/handle-structured-content-node.js | 28 +++++++++++- .../handle-structured-content-node.test.js | 44 +++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/packages/document-api/src/content-controls/content-controls.types.ts b/packages/document-api/src/content-controls/content-controls.types.ts index c1bf6570fc..64e946e536 100644 --- a/packages/document-api/src/content-controls/content-controls.types.ts +++ b/packages/document-api/src/content-controls/content-controls.types.ts @@ -124,10 +124,37 @@ export interface RepeatingSectionControlProperties { export interface ContentControlProperties { tag?: string; alias?: string; + /** + * Visual chrome behavior (``). + * + * Returned verbatim from the imported XML. When the source omits + * the element, this field is `undefined` — NOT silently set to + * `boundingBox`. Word's effective default when the element is + * absent is `boundingBox`, but consumers building UI on top of + * appearance (e.g. deciding whether to draw chrome) must apply + * that default themselves; the API does not fabricate it. + * + * Contract: + * - `'boundingBox'` → explicit; show chrome + * - `'tags'` → explicit; show tag markers + * - `'hidden'` → explicit; render transparently + * - `undefined` → source XML omitted the element; treat as + * Word's effective default (`'boundingBox'`). + */ appearance?: ContentControlAppearance; color?: string; placeholder?: string; showingPlaceholder?: boolean; + /** + * `` toggle (ECMA-376 §17.5.2.43). Word removes the + * SDT once the user fills it in. + * + * Returned verbatim from the imported XML: + * - `true` → element present (`` or `w:val="true"`/`"1"`) + * - `false` → element present with `w:val="false"`/`"0"` + * - `undefined` → element absent in source; treat as Word's + * effective default (`false`). + */ temporary?: boolean; tabIndex?: number; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js index a0154806cf..2b3c715ece 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js @@ -56,6 +56,27 @@ function extractPlaceholder(sdtPr) { return docPart?.attributes?.['w:val'] ?? null; } +/** + * Extract the `` toggle from sdtPr (ECMA-376 §17.5.2.43). + * + * Word's toggle conventions apply: an empty element means `true`; an + * element with `w:val="false"` or `w:val="0"` means `false`. The element + * being absent means the property was not specified — we return + * `undefined` so the Document API surfaces "absent" rather than fabricating + * the spec default. Consumers reading the property MUST treat undefined + * as "use the application default" (which for Word is `false`). + * + * @param {Object|null} sdtPr + * @returns {boolean|undefined} + */ +function extractTemporary(sdtPr) { + const el = sdtPr?.elements?.find((e) => e.name === 'w:temporary'); + if (!el) return undefined; + const val = el.attributes?.['w:val']; + if (val == null) return true; + return val !== '0' && val !== 'false'; +} + /** * @param {Object} params * @returns {Object|null} @@ -84,9 +105,10 @@ export function handleStructuredContentNode(params) { // Control type detection from sdtPr children const controlType = detectControlType(sdtPr); - // Appearance and placeholder + // Appearance, placeholder, and temporary toggle const appearance = extractAppearance(sdtPr); const placeholder = extractPlaceholder(sdtPr); + const temporary = extractTemporary(sdtPr); if (!sdtContent) { return null; @@ -117,6 +139,10 @@ export function handleStructuredContentNode(params) { type: controlType, appearance, placeholder, + // `temporary` is only set when the XML carries ``; + // omitted attrs stay undefined so consumers can distinguish + // "absent from source" from explicit false. + ...(temporary !== undefined ? { temporary } : {}), sdtPr, }, }; diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js index e2bad14387..db3ae3a9d3 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js @@ -225,6 +225,50 @@ describe('handleStructuredContentNode', () => { }); }); + describe('w:temporary parsing (SD-3111)', () => { + const parseTemporary = (sdtPrElements) => { + const node = createNode(sdtPrElements, [{ name: 'w:r', text: 'content' }]); + const params = { nodes: [node], nodeListHandler: mockNodeListHandler }; + parseAnnotationMarks.mockReturnValue({ marks: [] }); + return handleStructuredContentNode(params).attrs.temporary; + }; + + it('reads as true (empty toggle)', () => { + expect(parseTemporary([{ name: 'w:temporary' }])).toBe(true); + }); + + it('reads as true', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'true' } }])).toBe(true); + }); + + it('reads as true', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': '1' } }])).toBe(true); + }); + + it('reads as false', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'false' } }])).toBe(false); + }); + + it('reads as false', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': '0' } }])).toBe(false); + }); + + it('returns undefined (not false) when is absent', () => { + // Spec contract: absent in source XML stays undefined so consumers + // can distinguish "Word's effective default" from "explicit false". + expect(parseTemporary([])).toBeUndefined(); + expect(parseTemporary([{ name: 'w:tag', attributes: { 'w:val': 'unrelated' } }])).toBeUndefined(); + }); + + it('does not stamp temporary on attrs when absent (preserves "undefined" semantics)', () => { + const node = createNode([], [{ name: 'w:r', text: 'content' }]); + const params = { nodes: [node], nodeListHandler: mockNodeListHandler }; + parseAnnotationMarks.mockReturnValue({ marks: [] }); + const result = handleStructuredContentNode(params); + expect('temporary' in result.attrs).toBe(false); + }); + }); + describe('controlType detection', () => { const detectFrom = (sdtPrElements) => { const node = createNode(sdtPrElements, [{ name: 'w:r', text: 'content' }]); From 17b96dc99f86ab031748daa20c3c28d87eb4cf08 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 10:31:12 -0300 Subject: [PATCH 2/3] 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. --- .../src/content-controls/content-controls.types.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/document-api/src/content-controls/content-controls.types.ts b/packages/document-api/src/content-controls/content-controls.types.ts index 64e946e536..a8bb6f1c37 100644 --- a/packages/document-api/src/content-controls/content-controls.types.ts +++ b/packages/document-api/src/content-controls/content-controls.types.ts @@ -146,8 +146,10 @@ export interface ContentControlProperties { placeholder?: string; showingPlaceholder?: boolean; /** - * `` toggle (ECMA-376 §17.5.2.43). Word removes the - * SDT once the user fills it in. + * `` toggle (ECMA-376 §17.5.2.43). + * + * When enabled, Word treats the content control as temporary and may + * remove the SDT wrapper after the user edits/fills the control. * * Returned verbatim from the imported XML: * - `true` → element present (`` or `w:val="true"`/`"1"`) From 4ef6d95b17c9c6089b30917ccc7960e0dd5b57af Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 11:06:06 -0300 Subject: [PATCH 3/3] fix(super-editor): route extractTemporary through shared ST_OnOff parser (SD-3111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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: - → true - → false (the bug) - invalid token → undefined Verified: - 32/32 in handle-structured-content-node.test.js - 137/137 across the SDT handler suite --- .../helpers/handle-structured-content-node.js | 16 +++++++--------- .../handle-structured-content-node.test.js | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js index 2b3c715ece..c3d0232104 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js @@ -1,4 +1,5 @@ import { parseAnnotationMarks } from './handle-annotation-node'; +import { parseStrictStOnOff } from '../../../utils.js'; /** * Detect the semantic control type from sdtPr child elements. @@ -59,12 +60,11 @@ function extractPlaceholder(sdtPr) { /** * Extract the `` toggle from sdtPr (ECMA-376 §17.5.2.43). * - * Word's toggle conventions apply: an empty element means `true`; an - * element with `w:val="false"` or `w:val="0"` means `false`. The element - * being absent means the property was not specified — we return - * `undefined` so the Document API surfaces "absent" rather than fabricating - * the spec default. Consumers reading the property MUST treat undefined - * as "use the application default" (which for Word is `false`). + * Delegates to `parseStrictStOnOff` so token recognition matches the + * project's shared ST_OnOff convention (`true`/`1`/`on` → true; + * `false`/`0`/`off` → false). Returns `undefined` when the element is + * absent or carries an invalid token, preserving the "absent vs explicit + * false" distinction at the Document API surface. * * @param {Object|null} sdtPr * @returns {boolean|undefined} @@ -72,9 +72,7 @@ function extractPlaceholder(sdtPr) { function extractTemporary(sdtPr) { const el = sdtPr?.elements?.find((e) => e.name === 'w:temporary'); if (!el) return undefined; - const val = el.attributes?.['w:val']; - if (val == null) return true; - return val !== '0' && val !== 'false'; + return parseStrictStOnOff(el.attributes?.['w:val'], 'temporary', 'w:temporary'); } /** diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js index db3ae3a9d3..5f32162789 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js @@ -253,6 +253,20 @@ describe('handleStructuredContentNode', () => { expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': '0' } }])).toBe(false); }); + it('reads as true (ST_OnOff alias)', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'on' } }])).toBe(true); + }); + + it('reads as false (ST_OnOff alias)', () => { + // Without going through the shared ST_OnOff set this would + // incorrectly fall through to true. See utils.js parseStrictStOnOff. + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'off' } }])).toBe(false); + }); + + it('returns undefined for invalid w:val tokens (parser rejects unknown tokens)', () => { + expect(parseTemporary([{ name: 'w:temporary', attributes: { 'w:val': 'banana' } }])).toBeUndefined(); + }); + it('returns undefined (not false) when is absent', () => { // Spec contract: absent in source XML stays undefined so consumers // can distinguish "Word's effective default" from "explicit false".