Enter/Esc should not click Default/CancelControl if an input editor popup is open
Original Reporter info from Mantis: zpeterson @boramis
-
Reporter name: Zoë Peterson
Original Reporter info from Mantis: zpeterson @boramis
- Reporter name: Zoë Peterson
Description:
This patch fixes issue (a) mentioned in bug 35449:
If the keyboard input method editor (IME) is showing a popup window, Return and Escape should accept/cancel that and not click the form's Default/CancelControls.
It's possible to show the input editor popup on an English keyboard by setting focus to a TEdit/TMemo and then holding down a character that can have accents, like "i". The popup allows you to select between them. This also affects keyboards like Japanese/Hiragana, making them much more difficult to use.
Also fixes a hang when using the NumPad Enter key to click an Ok button.
Steps to reproduce:
- Open a modal dialog that contains an edit. Set focus in the edit.
- Press and hold the 'a' key until a popup is displayed.
- Press Esc
If the dialog has a Cancel button it will be clicked instead of just dismissing the dialog. Return will likewise click the Default button if the edit is a TEdit or TMemo with WantReturns=False.
Additional information:
This change is a lot more complicated than I'd like, but I haven't found a way to cleanly make it smaller. I've again split it into multiple smaller patches:
---
&LtPos;b>cocoa-ime-1.patch&LtPos;/b>
Added NSEvent.lclKeyRawChar to check key equivalents easier
This adds an objccategory to NSEvent that includes an lclKeyRawChar function, which returns charactersIgnoringModifiers if that string is 1 character long, or #0 in any other case. This isn't strictly required, but simplifies code that checks key equivalents. This is better than checking the NSEvent's keyCode directly, since kVK_Return and kVK_ANSI_KeypadEnter both map to #13 (closed) when matching key equivalents and characters like 'a' won't necessarily be on kVK_Ansi_A.
---
&LtPos;b>cocoa-ime-2.patch&LtPos;/b>
LCLCOCOA: Moved all "key has been handled" behavior into TLCLCommonCallback
This code moves all of the "key has been handled, stop further processing" logic into TLCLCommonCallback, instead of leaving it spread between the callback, TCocoaApplication, and TCocoaWindow. This is mostly necessary so patch 3 can call KeyEvHandled if the widget's KeyDown needs to swallow it, but does simplify the logic elsewhere.
It also fixes two issues:
- Marking a key as handled in UTF8KeyPress would still call the widget's keyDown.
- Marking a key as handled in LM_KEYDOWN/LM_SYSKEYDOWN, UTF8KeyPress, or CN_CHAR would still send the LM_CHAR message.
---
&LtPos;b>cocoa-ime-3.patch&LtPos;/b>
This is the actual fix.
&LtPos;b>** WARNING **&LtPos;/b> This introduces an external change in behavior. LM_KEYDOWN now always occurs in KeyEvAfterDown, after IntfUTF8KeyPress, CN_CHAR, and the Cocoa object's keyDown.
The core problem is that the NSTextView must process the key before we can know whether to send LM_KEYDOWN. The NSTextView's keyDown will pass the event to NSInputContext, which will either send it to the IME or send it to the edit via doCommandBySelector. Focus remains on the edit the entire time, and NSInputContext doesn't expose whether the IME popup is visible.
TCocoaTextView and TCocoaFieldEditor's keyDown allows the event through, and sets a flag if Return/Esc make it to doCommandBySelector. If it does, the event hasn't been captured and we can process it like normal. If it has, it's marked as handled. Once that's done, we can safely send LM_KEYDOWN to the LCL, which will set things up to press Default/CancelControl in the later keyUp.
Since Win32 is the only widgetset who's underlying API supports both WM_KEYDOWN and WM_CHAR, the widgetsets are all a different in the order messages are sent:
Win32: CN_KEYDOWN/widget/LM_KEYDOWN/CN_CHAR/widget/LM_CHAR Qt5: CN_KEYDOWN/LM_KEYDOWN/CN_CHAR/LM_CHAR/widget Gtk2: CN_KEYDOWN/CN_CHAR/widget/LM_KEYDOWN/LM_CHAR Old Cocoa: if SendChar CN_KEYDOWN/LM_KEYDOWN/CN_CHAR/widget/LM_CHAR else CN_KEYDOWN/widget/LM_KEYDOWN
This patch makes the Cocoa widgetset match the Gtk2 one. The CN_ messages all occur before keyDown, the LM_ ones are all after.
That works, except for one issue: If TCustomMemo's WantReturns property is false, its UTF8KeyPress (CN_CHAR) handler marks Enter as handled to avoid inserting a newline. Under GTK2 that suppresses the LM_KEYDOWN, which would break DefaultControl handling, but it works anyway, I think accidentally, because it always sends LM_KEYDOWN for Enter on text views even if they've been handled (gtk2procs.inc::EmulateEatenKeys).
To work around the above, I had to add a ForceReturnKeyDown boolean to TLCLCommonCallback that will let the keyDown through even if IntfUTF8KeyPress/CN_CHAR marks it as handled, and I had to implement TCocoaWSCustomMemo.SetWantReturns to suppress insertNewLine manually.
As part of the change, this also fixes a hang when using the NumPad Enter key in a TEdit. The existing TCocoaFieldEditor.keyDown only checked for kVK_Return, so it still allowed textDidEndEditing to go into the (still unsolved) endless "end-editing" loop. This is now protected by suppressing insertNewLine, which will keep it from triggering regardless of how it was activated. I did not include the redirected selectAll: call since TEdit has an "AutoSelect" property that already handles selecting the contents of the edit when pressing Enter, so doing it manually was unnecessary and broke the "AutoSelect = False" case.
---
Alternatives:
-
Some of the complexity of this patch is due to compatibility concerns with other controls (e.g. TSynEdit). It could be made smaller by only having the alternative message order used for TEdit and TMemo with WantsReturns = False. While I think that ForceReturnKeyDown is a bit of a hack I think it's better to have all of the controls go through the same code path and to have LM_KEYDOWN always occur after the widget sees it. This alternative feels much messier both in behavior and maintainability.
-
Instead of adding a wantReturns property to TCocoaTextView, ICommonCallback could expose a new "KeyEvCharSuppressed" function that just returns "not _SendChar". TCocoaTextView.insertNewLine would then check that instead. This is a bit less code, but it seemed like a less desirable approach since it relies on the callback's internal KeyEv state.
Mantis conversion info:
- Mantis ID: 35538
- Version: 2.0.3 (SVN)
- Monitored by: » @zeljan1 (Zeljan Rikalo)