Skip to content

fix(desktop): prevent browser default behavior for terminal shortcuts#988

Merged
Kitenite merged 1 commit into
superset-sh:mainfrom
onevcat:fix/terminal-ime-cursor-navigation
Feb 4, 2026
Merged

fix(desktop): prevent browser default behavior for terminal shortcuts#988
Kitenite merged 1 commit into
superset-sh:mainfrom
onevcat:fix/terminal-ime-cursor-navigation

Conversation

@onevcat
Copy link
Copy Markdown
Contributor

@onevcat onevcat commented Jan 27, 2026

Summary

Fix IME (Input Method Editor) input corruption after using Cmd+Left, Cmd+Right, Cmd+Backspace, or Shift+Enter shortcuts in the terminal.

Problem

When using IME to input CJK characters (Chinese, Japanese, Korean) in the terminal:

  1. Type some characters, e.g., "一二三四五" (Chinese for 1-2-3-4-5)
  2. Press Cmd+Left to move cursor to the beginning of the line
  3. Try to type any new character
  4. Bug: The input becomes corrupted - in this case, any input would result in just "五" (the last character)

As is

2026-01-27.12.43.52.mov

To be

2026-01-27.12.44.45.mov

More detail

This issue does not occur when:

  • Using Ctrl+A directly (instead of Cmd+Left)
  • Typing non-IME characters (English, numbers)
  • Using arrow keys to move cursor character by character

Root Cause

This bug was introduced in #926 (feat(terminal): add Cmd+Arrow for cursor line navigation), which remapped shortcuts to match macOS conventions:

Shortcut Before #926 After #926
Terminal switching Cmd+Left/Right Cmd+Option+Left/Right
Pane switching Cmd+Option+Left/Right Cmd+Shift+Left/Right
Cursor line navigation (none) Cmd+Left/Right

xterm.js uses a hidden <textarea> element to capture keyboard input and IME composition. When handling custom shortcuts via attachCustomKeyEventHandler:

  • Returning false tells xterm to skip its default key handling
  • However, it does not call event.preventDefault(), so browser default behavior still occurs
  • Cmd+Left in browsers moves the textarea cursor to the beginning of the text
  • This cursor movement in the hidden textarea interferes with IME's internal composition state

Solution

Add event.preventDefault() before handling the following shortcuts:

  • Cmd+Left (move to line beginning)
  • Cmd+Right (move to line end)
  • Cmd+Backspace (delete to line beginning)
  • Shift+Enter (line continuation)

This prevents the browser from manipulating the hidden textarea's cursor position while we send the appropriate control sequences to the PTY.

Testing

  1. Open terminal in Superset desktop app
  2. Switch to an IME (e.g., Chinese Pinyin, Japanese)
  3. Type some CJK characters
  4. Press Cmd+Left to jump to line beginning
  5. Type more characters
  6. Expected: New characters appear at cursor position normally
  7. Before fix: Input is corrupted

Additional Notes

A video demonstration can be provided if needed to illustrate the bug and the fix.

Add event.preventDefault() for Cmd+Left, Cmd+Right, Cmd+Backspace, and
Shift+Enter to prevent browser default actions from interfering with
xterm's hidden textarea. This fixes a bug where IME input would
malfunction after using these shortcuts for cursor navigation.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 27, 2026

📝 Walkthrough

Walkthrough

This PR adds event.preventDefault() calls to four keyboard event handlers in the Terminal component (Shift+Enter, Cmd+Backspace, Cmd+Left, Cmd+Right), preventing browser default behavior when these keys are pressed while the terminal handles them.

Changes

Cohort / File(s) Summary
Terminal keyboard event prevention
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
Added event.preventDefault() calls in four keyboard-handling branches within setupKeyboardHandler to block browser defaults for Shift+Enter, Cmd+Backspace, Cmd+Left, and Cmd+Right key combinations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • Kitenite

Poem

🐰 Four keys pressed, defaults gone to sleep,
Cmd and Shift now secrets we shall keep,
No browser interference, just our way,
The terminal hops free, hip-hip-hooray! 🌿

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately reflects the main change: adding event.preventDefault() calls to prevent browser default behavior for terminal shortcuts.
Description check ✅ Passed The pull request description is comprehensive and well-structured, covering the problem, root cause, solution, testing steps, and additional context, exceeding template requirements.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

@Kitenite Kitenite merged commit ec026df into superset-sh:main Feb 4, 2026
1 check passed
@Kitenite
Copy link
Copy Markdown
Collaborator

Kitenite commented Feb 4, 2026

Thanks for creating the PR, sorry I must've missed this entirely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants