Skip to content

attachCustomKeyEventHandler inverts xterm.js's return-value contract (silently swallows all input) #192

Description

@kolapapa

Summary

attachCustomKeyEventHandler returns the opposite of what xterm.js's handler of the same name returns. Because the README's migration story is "change your import: @xterm/xtermghostty-web", an app that carries its existing handler across ends up with a terminal that ignores every keystroke — while connecting, rendering, scrolling and showing the shell prompt perfectly. There is no error and nothing on screen to suggest the keyboard is the problem.

The two contracts

xterm.js — the return value says let the terminal have this key:

The function returns whether the event should be processed by xterm.js.

So a handler that only wants to claim a shortcut or two returns true for everything else.

ghostty-web — the return value says I already handled it, drop it (lib/input-handler.ts#L377-L385):

// Check custom key event handler
if (this.customKeyEventHandler) {
  const handled = this.customKeyEventHandler(event);
  if (handled) {
    // Custom handler consumed the event
    event.preventDefault();
    return;
  }
}

That same xterm-shaped handler now returns true for every ordinary key, so every ordinary key is swallowed — and the one or two keys it deliberately claimed (returning false) are the only ones that reach the pty, inverted in both directions.

Reproduction

import { init, Terminal } from 'ghostty-web';

await init();
const term = new Terminal();
term.open(el);
term.onData(d => ws.send(d));

// Straight from an xterm.js app: claim Ctrl+F, let everything else through.
term.attachCustomKeyEventHandler((e) => {
  if (e.ctrlKey && e.key === 'f') { e.preventDefault(); return false; }
  return true;
});

Expected: typing works, Ctrl+F is swallowed.
Actual: nothing can be typed at all, and Ctrl+F is the only key that reaches the shell.

Removing the handler entirely makes typing work again, which is what makes this hard to attribute — the handler is usually old, unrelated code that was correct before the migration.

Versions

ghostty-web@0.4.0, and the code above is current main.

Suggestion

Either is fine from where I sit, but the ambiguity is the expensive part:

  1. Invert it to match xterm.js — treat a falsy return as "consumed". Most faithful to the stated API compatibility, but silently changes behaviour for anyone who already adapted, so it would want a note in the changelog.
  2. Keep the current meaning and document it — call it out in the README migration line and in the JSDoc on attachCustomKeyEventHandler, since that method is the one place a reader would look. Today the JSDoc says "Returns true to prevent default handling", which is accurate but only visible if you already suspect the handler.

A line in the compatibility table would also have caught this for me before I shipped it.

Happy to send a PR for whichever direction you prefer.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions