gh-55646: Do not crash IDLE on an invalid key binding - #152747
serhiy-storchaka wants to merge 5 commits into
Conversation
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.
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
left a comment
There was a problem hiding this comment.
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?
|
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 <[email protected]>
|
@terryjreedy, does my answer above address your question? Every configured key binding does go through MultiCall, so it is the single place where an invalid sequence can be caught, and it also catches sequences that the config code cannot check without a Tk root. The same kind of problem exists for other config values. An invalid color in a user theme crashes the Settings dialog (#85604). A config file that cannot be parsed at all (a syntax or encoding error) was handled in #152764. Fonts are already tolerant: a bad size falls back to the default with a warning, and Tk substitutes an unknown family. |
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_addstores the parsed sequence without validating the keysym, so theTclErroris raised later, when a virtual event is bound and MultiCall performs the actual Tkbind. With no window yet, IDLE just exits.Catch
TclErrorat both binder call sites (MultiCall.bindandMultiCall.event_add) and ignore the invalid binding with a warning naming it. Thebindpath also drops the bad sequence from the virtual event, which avoids a secondaryValueErrorif it is later rebound or unbound.🤖 Generated with Claude Code