fix(super-editor): render and round-trip w:smartTag content (SD-2647)#3546
Conversation
w:smartTag is a transparent inline wrapper around EG_PContent (ECMA-376 §17.5.1.9). Before this change it fell through to the v2 passthrough path, which wrapped the runs in a display:none span - so any content authored inside Word's smart tags (countries, places, dates, names) disappeared on render. Add a smartTag PM container node (non-atomic, content: inline*, transparent renderDOM) plus a v3 translator that preserves w:element, w:uri, and the optional w:smartTagPr child for round-trip. A small v2 importer entity intercepts w:smartTag before the passthrough catcher and delegates to the v3 translator. The pm-adapter inline converter mirrors structuredContent / bookmarkStart: walk children with inherited marks and forward inlineRunProperties so SD-2781 run-level bidi/script metadata survives. Sdt run-flattening recognises w:smartTag as a run-level wrapper so nested smartTags inside SDT content keep working. Container node, not a mark: smartTags nest in real documents (e.g. <w:smartTag element="place"><w:smartTag element="country-region">), and ProseMirror marks of the same type don't stack. Verified on the IT-945 customer file: 439 smartTag elements render (matches 439 XML opens), 22/22 sampled country names visible, 0 passthroughInline wrappers. Tests: 11 translator + 4 extension; full super-editor / pm-adapter / layout-engine suites green; workspace check:types clean.
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
|
The ecma-spec MCP tools keep getting denied — I'll proceed from ECMA-376 §17.5.1.9 directly. Status: PASS I reviewed the smartTag translator against ECMA-376 §17.5.1.9 (CT_SmartTagRun) and the PR is spec-compliant. What I checked:
No non-existent attributes, no missing required-attr handling that would break valid input, no incorrect defaults, no spec violations. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c08072536
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
2 issues found across 14 files
Linked issue analysis
Linked issue: SD-2647: Bug: Render and round-trip content wrapped in w:smartTag
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add smartTag ProseMirror node (inline non-atomic container with attrs element, uri, smartTagPr and transparent renderDOM) | The PM node implementation, attributes and renderDOM placeholder are present and unit-tested. |
| ✅ | Add w:smartTag v3 translator (encode/decode, preserve w:element/w:uri and capture/re-emit , route full EG_PContent) | Translator implemention includes encode/decode, attribute handlers, smartTagPr capture/re-emit and has unit tests covering attribute mapping and smartTagPr round-trip. |
| ✅ | Add pm-adapter inline converter and register it so smartTag children are visited into runs | A smart-tag inline converter was added and registered in the paragraph converter registry, mirroring existing structured-content handling. |
| ✅ | Add importer handler to capture before passthroughInline | Importer handler for w:smartTag was added and wired into the default node list handler so smartTag no longer falls through to passthroughInline. |
| ✅ | Exporter emits and re-emits preserved when present | Decoder emits w:smartTag with decoded attributes and re-inserts cloned smartTagPr when present; tests assert the smartTagPr is re-emitted and cloned (not same reference). |
| ✅ | Update SDT flatten helper to include w:smartTag so SDT-nested smartTags flatten correctly | RUN_LEVEL_WRAPPERS was extended to include 'w:smartTag'. |
| Visible content wrapped by w:smartTag renders correctly in paragraphs / tables / headers & footers / inside SDTs; nested smartTags preserve both wrappers | The code changes implement transparent rendering and pm-adapter/import pipeline changes needed for these contexts. Unit tests are present for node behavior and translator; however there are no end-to-end integration tests in the diff explicitly exercising paragraphs/tables/headers/footers or SDT nesting. The PR body reports manual verification (IT-945) showing sampled country names render and passthroughInline counts drop to 0. | |
| ✅ | Round-trip export/import preserves w:element, w:uri and | Translator encode/decode explicitly map attributes and preserve smartTagPr; unit tests validate attribute mapping and that smartTagPr is re-emitted as a clone, covering the round-trip logic at translator level. |
| ❌ | Exported .docx opens in Microsoft Word without a corruption prompt | No automated integration test or explicit export-into-.docx + Word-open verification is present in the diff. The PR description includes manual rendering checks but does not explicitly state Word-open results for exported files. |
| ✅ | No regression: passthroughInline remains atomic + hidden and unknown content is still preserved by passthroughInline | The PR leaves passthrough implementation untouched and adds a handler to avoid routing smartTag content into passthrough. The PR description reports passthroughInline wrapper count dropping to 0 in manual checks, indicating no regression and that visible content is no longer hidden behind passthroughInline. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…gPr in SDT flatten (SD-2647) The initial commit wired w:smartTag for import + render but left two holes on the export path: 1. exporter.js's exportSchemaToJson dispatch table had no smartTag entry, so the v3 decoder was unreachable. Any imported paragraph containing a smartTag would round-trip to null and the entire wrapped subtree would be dropped on save. 2. convertSdtContentToRuns added w:smartTag to RUN_LEVEL_WRAPPERS but then recursed on the full element list, so a w:smartTagPr nested inside an SDT-wrapped smartTag fell through the recursion's catch-all and was rewrapped as a fake <w:r>. That produces invalid OOXML and corrupts the smartTagPr round-trip in that nesting. Fixes both: - Import wSmartTagNodeTranslator and register it under `smartTag` in the export router. - Partition w:smartTagPr out of the recursive call when the wrapper is w:smartTag, and prepend it back to the wrapper's elements. Kept scoped to the w:smartTag + w:smartTagPr pair; other run-level wrappers are unaffected. New tests: - smartTag-export-routing.test.js exercises exportSchemaToJson directly so the routing wiring cannot regress. - convert-sdt-content-to-runs.test.js asserts w:smartTagPr stays as smartTag metadata instead of becoming a fake w:r. Both new tests fail on the previous commit and pass with this fix. super-editor: 13525 pass / 13 skip. Workspace check:types clean.
…trip (SD-2647) The previous export-routing tests proved the wrapper and attrs reach the XML, but didn't assert that the child runs survived. Add a tiny recursive text collector and assert the inner "Brazil" text is present in both the bare and smartTagPr-preserving cases, so a future regression that loses the smartTag content (without losing the wrapper) would still fail the test.
… (SD-2647) Two test gaps surfaced during PR review that were not blocking by themselves but together left the customer-bug surface unprotected. smartTagImporter.test.js pins the v2 importer bridge invariant: the passthrough handler refuses any node present in v3 registeredHandlers, so without the smartTagNodeEntityHandler claiming w:smartTag, the node silently falls off the end of the reducer chain. Cubic and Codex both read this as "duplicate registration"; the regression-guard test makes the bridge's necessity explicit and reproducible. The behavior round-trip spec exercises the full chain end-to-end with a sanitized real-Word fixture (OOXML surgery on the IT-945 customer .docx, not regenerated through SuperDoc's save path, so the input does not launder through the implementation under test). Asserts: - input fixture already carries w:tc / w:smartTag / w:element / w:smartTagPr (sanity gate before assertions), - table-cell country names render visibly (the customer bug surface, previously hidden as passthroughInline), - zero-edit exportDocx output still contains w:smartTag with the original w:element, the inner country text, and w:smartTagPr. Both tests run green against the SD-2647 fix; the round-trip spec takes ~4s, the importer unit ~2ms. Deferred per review: pm-adapter forwarding test (only common.test.ts currently asserts inlineRunProperties at the helper level; structured- content and other transparent wrappers have no forwarding tests, so a smartTag-only forwarder would set a new bar inconsistent with peers). Layout snapshot + visual pixel tests deferred too; smartTag is transparent metadata, the visible-text behavior assertion already covers what would change.
caio-pizzol
deleted the
caio/sd-2647-bug-render-and-round-trip-content-wrapped-in-wsmarttag
branch
2 participants
Summary
smartTagPM container node for<w:smartTag>so children render instead of falling into hiddenpassthroughInline.w:element,w:uri, andw:smartTagPr.Verification
pnpm check:types: cleanpassthroughInlinecount drops to 0Linear: SD-2647 (under SD-3298). Design rationale in the ticket.