[Patch] Cocoa: Rewrite keyDown/keyUp event handling to fix various issues
Original Reporter info from Mantis: zpeterson @boramis
-
Reporter name: Zoë Peterson
Original Reporter info from Mantis: zpeterson @boramis
- Reporter name: Zoë Peterson
Description:
The attached patch replaces much of the key handling code in the NSResponder descendants (TCocoaWindow/TCocoaWindowContent/TCocoaButton/etc). The TLCLCallback objects are largely unchanged.
The primary change is the removal of WindowPerformKeyDown and the per-control keyUp handlers, in favor of new blocks in TCocoaApplication.sendEvent and TCocoaWindow.keyDown.
I've summarized it below, but for the full reasoning, it will help to read through this WWDC presentation: https://asciiwwdc.com/2010/sessions/145
In short, Cocoa's key handling looks like this:
NSApplication.sendEvent Local NSEvent Monitors if modifierFlags include Cmd or Ctrl NSWindow.performKeyEquivalent (Key window) NSResponder.performKeyEquivalent (called on all views recursively) NSWindow.performKeyEquivalent (Other "active" windows, but only for views that manage menus) NSMenu.performKeyEquivalent Check registered hot keys (e.g., Cmd~ for Window cycling or Ctrl+F2 to move focus to menu) NSWindow.sendEvent (Key window) NSResponder.keyDown (firstResponder) inherited calls keyDown up the responder chain until it reaches the NSWindow NSWindow.keyDown NSWindow.performKeyEquivalent (Key Window, possibly for second time if Cmd or Ctrl) NSMenu.performKeyEquivalent (again, possibly the second time) &LtPos;undocumented magic> Beep
The existing code was wrong in several ways:
- It wasn't calling KeyEvBefore until NSWindow.sendEvent, which meant that performKeyEquivalent may have already triggered an action. There are comments about this in the existing code.
- It called performKeyEquivalent on the firstResponder rather than the window, and TCocoaWindowContent.performKeyEquivalent didn't call inherited in most cases, breaking what is supposed to be a recursive descent down through the views.
- TCocoaApplication.sendEvent sent some keyUp events to both the window and the through the inherited sendEvent, so they were processed twice.
- keyUp handling had a mix of calls to either KeyEvent or KeyEvBefore/KeyEvAfter, which results in a lot of duplicated code (to be removed in a later patch).
- NotifyApplicationUserInput was called after sending the CN_KEY* messages. The other widgetsets all call it before sending those.
- Pressing ESC to click a Cancel button would either beep as it was clicked or, if an edit/memo had focus, be ignored entirely.
From a high level:
A) TCocoaApplication.sendEvent captures all incoming NSKeyUp and NSKeyDown events and immediately sends them to the firstResponder's callback object. This allows us to handle them before anything else in the system. The first potential issue occurs here. Because we're doing this before the inherited NSApplication.sendEvent, local NSEvent monitors (NSEvent.addLocalMonitorForEvents) won't have a chance to run before we process it. For most LCL applications this won't have any effect, but it is something to be aware of.
B) KeyEvBefore* is mostly unchanged. I did move NotifyApplicationUserInput up so it occurs before the DeliverMessage(CN_KEYDOWN) call and doesn't occur for LN_KEY*/CN_CHAR/LM_CHAR, to match the Win32 behavior.
C) Once KeyEvBefore returns, unhandled events are sent through the inherited send event. Cocoa calls the firstResponder's keyDown/keyUp method, and calls to inherited will keep moving it up the responder chain until it gets to TCocoaWindow.keyDown. Any keys that Cocoa or a widget handles won't call inherited, so they won't reach TCocoaWindow.
D) TCocoaWindow.keyDown is similar to the existing code in WindowPerformKeyDown. First, it calls performKeyEquivalent on itself and the main menu. This will recursively go down through TCocoaWindowContent and the rest of the views.
E) TCocoaWindowContent.performKeyEquivalent marks [Return] and [Esc] as handled if the associated form has a DefaultControl or CancelControl. I'm not happy about the duplication between this code and TApplication.DoEscapeKey/DoReturnKey (see my mailing list post), but it's necessary to prevent other views from handling the keys and to stop the beep. I did add a performKeyEquivalent to TCocoaButton to avoid it being clicked if it's the default key, but that shouldn't trigger. As a an alternative, TCococaButton could just always return false from performKeyEquivalent, but I wanted to leave open the option of someone setting their own keyEquivalents outside [Return].
F) TCocoaWindowContent.performKeyEquivalent now calls its own inherited to continue the recursive descent. I think the NSResponderHotKeys block could be cleaned up a bit more, and as the comment says, it could be moved to TCocoaFieldEditor and TCocoaTextView, but that isn't necessary for this patch.
G) Once performKeyEquivalent returns, TCocoaWindow.keyDown calls KeyEvAfterDown and then calls its own inherited if KeyEvAfterDown doesn't handle the key. The inherited keyDown will call performKeyEquivalent again, but it won't affect anything. The inherited call is also where beeps occur for unhandled keys, which is why KeyEvAfterDown is called first. Not calling the inherited method to avoid the beep isn't an option. Even though the first responder and all of its parent views have seen the keyDown, and performKeyEquivalent has been called, there's additional undocumented/private behavior we can't replicate. For example, without it, using spacebar to toggle a checkbox doesn't work.
H) Once the keyDown event has been fully handled, control returns to TCocoaApplication.sendEvent, where we call KeyEvAfter if it didn't make it all the way up to TCocoaWindow.keyDown.
Comments and concerns:
a) In an edit in a modal dialog, if you hold down a key for a character that may have accents (e.g, 'i'), the system will show a popup allowing you to pick an accented variant. Pressing Return or Esc should only affect that popup, but it currently will also trigger the Default/Cancel buttons, closing the dialog. This also affects input methods that rely on popup windows (e.g. Japanese/Hiragana). This also affects the existing code, though it was less noticable since that ignored ESC when a TEdit/TMemo had focus. As long as DoEscapeKey/DoReturnKey are done in LM_KEYUP instead of LM_KEYDOWN, I don't see how to work around it. Combobox popups are not affected in the same way because they use a nested message loop.
b) CN_KEYDOWN now calls TCustomForm.IsShortCut and TMainMenu.IsShortCut before the main menu's performKeyEquivalent occurs. Previously this varied depending on whether the shortcut included Cmd/Ctrl or not. The side effect of this is that the menubar won't flash to show which submenu was activated. In my opinion, the best way to solve this would be for TMenu.IsShortCut to call a new TWSMenu.IsShortCut function that can call menu.performKeyEquivalent internally. This is a nicety and isn't necessary for correct behavior.
c) This doesn't do anything with handleFlagsChanged. I intend to do that next, along with removing TLCLCommonCallback.KeyEvent.
d) I don't like the addition of _keyEvCallback and _calledKeyEvAfter to TCocoaWindow, but I don't see an alternative. I don't think it would be a good idea to ask the first responder for its callback again in case that's changed. I also don't like the duplication with DoEscapeKey/HasCancelControl and DoReturnKey/HasDefaultControl.
Steps to reproduce:
The attached project is mostly just for testing various keyboard handling things. The edit on the left (and the console, if run from the command line) will print out all of the different keyboard events as they're triggered. For testing, I ran it on both Windows and Cocoa and compared the output when pressing various key combinations.
Mantis conversion info:
- Mantis ID: 35449
- Version: 2.0.3 (SVN)
- Fixed in revision: 61106 (#ae91507d)