Skip to content

fix: stop undo propagation in mask editor and wire maskeditor_is_opended#10222

Open
Doomeriss wants to merge 3 commits intoComfy-Org:mainfrom
Doomeriss:fix/mask-editor-ctrlz-bug
Open

fix: stop undo propagation in mask editor and wire maskeditor_is_opended#10222
Doomeriss wants to merge 3 commits intoComfy-Org:mainfrom
Doomeriss:fix/mask-editor-ctrlz-bug

Conversation

@Doomeriss
Copy link

@Doomeriss Doomeriss commented Mar 17, 2026

Summary

Fix Ctrl+Z in mask editor undoing the graph state and reverting pasted images in Load Image node.

Changes

  • What: Added event.stopPropagation() and event.preventDefault() to Ctrl+Z/Y handlers in useKeyboard.ts to prevent undo events from bubbling up to LiteGraph. Also wired ComfyApp.maskeditor_is_opended to the existing isOpened() function in maskeditor.ts which was never assigned, causing the changeTracker to always run undo even when the mask editor was open.

Review Focus

The maskeditor_is_opended static property was defined in app.ts and checked in changeTracker.ts but never actually assigned in maskeditor.ts during the extension init. This meant the guard that prevents LiteGraph undo while the mask editor is open was effectively dead code.

Screenshots (if applicable)

Steps to reproduce the fixed bug:

  1. Paste an image into a Load Image node
  2. Open the mask editor
  3. Paint something
  4. Press Ctrl+Z
  5. Before fix: pasted image reverts to previous state / image.png
  6. After fix: only the brush stroke is undone not the image pasted aswell

┆Issue is synchronized with this Notion page by Unito

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

🎨 Storybook: loading Building...

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 382c6f6c-09b7-4ff8-abdf-ec39809d5f28

📥 Commits

Reviewing files that changed from the base of the PR and between 77646b2 and ab6d6da.

📒 Files selected for processing (1)
  • src/composables/maskeditor/useKeyboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/composables/maskeditor/useKeyboard.ts

📝 Walkthrough

Walkthrough

Adds prevention of default/browser propagation for Ctrl/Cmd+Y and Ctrl/Cmd+Z in the mask editor keyboard handler, and exposes ComfyApp.maskeditor_is_opended as a global compatibility alias bound to the mask editor's isOpened function.

Changes

Cohort / File(s) Summary
Keyboard Event Handling
src/composables/maskeditor/useKeyboard.ts
Added event.stopPropagation() and event.preventDefault() in the Ctrl/Cmd+Y (redo) and Ctrl/Cmd+Z (undo) branches to suppress default browser actions and stop event bubbling.
Global API Exposure
src/extensions/core/maskeditor.ts
Introduced global compatibility alias ComfyApp.maskeditor_is_opended assigned to the local isOpened function during extension initialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped to the keys with a bright little tune,
Ctrl and Cmd now behave in the moon,
No stray browser tricks to spoil the play,
And a new global name shows if I may —
A rabbit's small cheer for a tidy new day.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
End-To-End Regression Coverage For Fixes ❌ Error PR title uses bug-fix language ('fix:') triggering regression test requirement, but no files under browser_tests/ were modified and PR description lacks concrete explanation for why end-to-end regression test was not added. Add Playwright regression test under browser_tests/ reproducing the bug scenario, or add concrete explanation in PR description of why browser-based test is not practical.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes both main changes: stopping undo propagation in the mask editor and wiring the maskeditor_is_opended property.
Description check ✅ Passed The description covers all required sections with clear explanations of what was changed, why it matters, and includes reproduction steps demonstrating the fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

🎭 Playwright: ⏳ Running...

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants