fix(editor): prevent deletion of globalContent node on select-all + delete#3347
Open
fix(editor): prevent deletion of globalContent node on select-all + delete#3347
Conversation
…elete Add a ProseMirror appendTransaction plugin to the GlobalContent extension that automatically restores the globalContent node (with all its data, including theme configuration) when it is removed from the document. This fixes the issue where CMD+A followed by Delete would remove the globalContent node, causing the theme to be lost. The plugin detects when the globalContent node disappears from the document after a transaction that changed the doc, retrieves the previous node's attributes from the old state, and re-inserts it at position 0. The restoration transaction is marked as non-historical so it doesn't pollute the undo stack. Co-authored-by: Danilo Woznica <danilowoz@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 5/5
- This looks low risk to merge: the only finding is low severity (3/10) and appears limited to test behavior rather than production logic.
- In
packages/editor/src/extensions/global-content.spec.ts, usingTextSelectioninstead of the real select-all path may miss the leaf-block edge case, so the test may not fully validate the targeted bug scenario. - Pay close attention to
packages/editor/src/extensions/global-content.spec.ts- ensure the spec exercises the true select-all flow so the leaf-block edge case is covered.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/editor/src/extensions/global-content.spec.ts">
<violation number="1" location="packages/editor/src/extensions/global-content.spec.ts:46">
P3: Use the real select-all path here; `TextSelection` can miss the leaf-block edge case this bug targets.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }); | ||
|
|
||
| const { state } = ed; | ||
| const allSelection = TextSelection.create( |
Contributor
There was a problem hiding this comment.
P3: Use the real select-all path here; TextSelection can miss the leaf-block edge case this bug targets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/editor/src/extensions/global-content.spec.ts, line 46:
<comment>Use the real select-all path here; `TextSelection` can miss the leaf-block edge case this bug targets.</comment>
<file context>
@@ -0,0 +1,237 @@
+ });
+
+ const { state } = ed;
+ const allSelection = TextSelection.create(
+ state.doc,
+ 0,
</file context>
… patterns Adopt two patterns from the container enforcer plugin: 1. Collaboration-aware no-op guard: skip doc.eq(oldDoc) transactions from Liveblocks stabilisation rounds to avoid restoring the node before the real document arrives. 2. View-based restoration: in non-collaborative mode, use the plugin's view hook to track the last-known globalContent node and restore it on update, matching how containerEnforcer proactively checks on view init/update. Co-authored-by: Danilo Woznica <danilowoz@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When users press CMD+A (select all) followed by Delete/Backspace, the entire document content is replaced — including the
globalContentnode that stores the theme configuration (styles, theme name, CSS). This causes the theme to be permanently lost from the document.Solution
Added a ProseMirror
globalContentProtectorplugin to theGlobalContentextension, closely following the patterns established by thecontainerEnforcerplugin incontainer.tsx:appendTransaction— After any doc-changing transaction, checks if theglobalContentnode is still present. If it's missing but existed in the previous state, restores it at position 0 with all original attributes (theme, styles, CSS).doc.eq(oldDoc)transactions from Liveblocks stabilisation rounds to avoid premature restoration before real document content arrives (same heuristic used bycontainerEnforcer).view-based restoration — In non-collaborative mode, uses the plugin'sviewhook to track the last-knownglobalContentnode and restore it on update, matching howcontainerEnforcerproactively checks on view init/update.addToHistory: falseso it doesn't pollute the undo stack.Changes
packages/editor/src/extensions/global-content.ts— AddedaddProseMirrorPlugins()with theglobalContentProtectorpluginpackages/editor/src/extensions/global-content.spec.ts— New test file with 7 tests covering:replaceWithoperationssetGlobalContentcommand functionalitySlack Thread
Summary by cubic
Prevents loss of theme configuration by restoring the
globalContentnode if it’s removed (e.g., select-all + delete). Works in collab and non-collab sessions without polluting undo history.globalContentProtectorto reinsert the node at pos 0 with previous attrs; uses view-based restore in non-collab mode and skips no-opdoc.eq(oldDoc)transactions to avoid premature restores with Liveblocks.addToHistory: false; tests cover select-all delete, replace operations, no duplication, andsetGlobalContentupdates.Written for commit aba11d8. Summary will update on new commits.