Skip to content

gh-55646: Do not crash IDLE on an invalid key binding#152747

Open
serhiy-storchaka wants to merge 5 commits into
python:mainfrom
serhiy-storchaka:gh-55646-invalid-keybinding-crash
Open

gh-55646: Do not crash IDLE on an invalid key binding#152747
serhiy-storchaka wants to merge 5 commits into
python:mainfrom
serhiy-storchaka:gh-55646-invalid-keybinding-crash

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Jul 1, 2026

Copy link
Copy Markdown
Member

A typo in a custom key binding entered via Advanced Key Binding (which is not validated), such as <Alt-Key-up> instead of <Alt-Key-Up>, crashed IDLE at startup:

  File ".../idlelib/multicall.py", line ..., in bind
    self.__binders[triplet[1]].bind(triplet, func)
  ...
_tkinter.TclError: bad event type or keysym "up"

MultiCall.event_add stores the parsed sequence without validating the keysym, so the TclError is raised later, when a virtual event is bound and MultiCall performs the actual Tk bind. With no window yet, IDLE just exits.

Catch TclError at both binder call sites (MultiCall.bind and MultiCall.event_add) and ignore the invalid binding with a warning naming it. The bind path also drops the bad sequence from the virtual event, which avoids a secondary ValueError if it is later rebound or unbound.

🤖 Generated with Claude Code

A typo in a key binding, such as <Alt-Key-up> for <Alt-Key-Up>, crashed IDLE
at startup.  It is now ignored with a warning.
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jul 1, 2026
serhiy-storchaka and others added 2 commits July 1, 2026 18:07
On macOS an invalid binding such as <Alt-Key-up> is not parsed into a
MultiCall triplet, so it falls back to Tk's event_add(), which was left
unguarded and still crashed.  Guard the fallback too, and test both the
parsed (bind()) and unparsed (event_add()) paths on every platform.

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is an example of making the good the enemy of the adequate. Unexpected crashes/exits are bad and this is definately better than no fix.

The merge conflict was due to this being the 2nd patch to add tests at the end of test_multicall. The local interface had a button to accept both additions.

I also looked at Serwy's patch: https://bugs.python.org/file24351/issue11437.patch.
His concern about repeated processing is mostly moot for most users. We merged all the IDLE feature extensions into the main code about a decade ago, leaving only a test extension.

Main question: Why catch errors in multicall rather than configparser? Does every sequence get run through multicall? Does this catch more errors?

@serhiy-storchaka

Copy link
Copy Markdown
Member Author

Note that "Accept both" eats a blank line between fragments. You have to add it after pressing "Accept both" while you are in the online editor. I fixed this.

Yes, every configured key binding goes through MultiCall. MultiCall is the single choke point where a binding string is actually handed to Tk.

Serwy's patch also works. But it pulls a Tk root plus a modal dialog into config code that also runs headless (tests, the subprocess). Naming the bad key and suggesting a reconfigure is a nice touch, which I've added to the warning.

idlelib is private, so its helpers need no leading underscore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants