fix(document-api): allow Document API to update contentLocked SDTs#3718
fix(document-api): allow Document API to update contentLocked SDTs#3718mattConnHarbour wants to merge 2 commits into
Conversation
contentLocked SDTs block inner-content modification via the lock plugin, but the Document API should still be able to update them programmatically. Use whole-node replacement for contentLocked SDTs: replace the entire node (wrapper + content) instead of modifying inner positions. This preserves all attributes including the ID while bypassing the lock plugin's position checks. - Add assertNotFullyLocked() - rejects only sdtContentLocked (fully locked) - Add requiresWholeNodeReplacement() - returns true for contentLocked - Add replaceEntireSdt() - atomic replacement preserving original attrs - Update text.setValue, text.clearValue, replaceContent, clearContent SD-3429 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
f6fb383 to
5954387
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6fb3831be
ℹ️ 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".
| tr.replaceWith(from, to, updatedNode); | ||
| dispatchTransaction(editor, tr); | ||
| return true; |
There was a problem hiding this comment.
Report failure when whole-node replacement is filtered
When this new contentLocked path targets an SDT nested inside a locked ancestor, the structured-content lock plugin can reject the dispatched replacement transaction without throwing; replaceEntireSdt still returns true, and executeSdtMutation then reports a successful mutation even though the document did not change. This is especially reachable because nested content controls are supported by the API, so the whole-node path should verify that the dispatch actually changed the document or preflight ancestor locks before returning success.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Detect silent transaction rejection in replaceEntireSdt by comparing doc references before/after dispatch (fixes nested locked controls false-success path) - Add comprehensive regression tests for contentLocked whole-node replacement code path (SD-3429) - Document the semantic bypass intent in lock-enforcement.ts and structured-content-lock-plugin.js - Remove misleading "recovery" and "atomicity" comments Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
contentLockedSDTs block inner-content modification via the lock pluginChanges
assertNotFullyLocked()- rejects onlysdtContentLocked(fully locked)requiresWholeNodeReplacement()- returns true forcontentLockedreplaceEntireSdt()- atomic replacement preserving original attrstext.setValue,text.clearValue,replaceContent,clearContentto use whole-node replacement forcontentLockedSDTsLock mode behavior
unlockedsdtLockedcontentLockedsdtContentLockedTest plan
contentLockedSDTtext.setValueupdates it successfullysdtContentLockedSDTs are still rejectedFixes SD-3429
🤖 Generated with Claude Code