diff --git a/CHANGELOG.md b/CHANGELOG.md index 68de3c0eb..56638bdf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Core.Desktop] Added a constant (kBrowserCompatibleUserAgent) to RobustNetworkOperation: a browser-like User Agent string that can be used when making HTTP requests to strict servers. - [SIL.Core] Added an Exception property to NonFatalErrorReportExpected to return the previous reported non-fatal exception. - [SIL.Media] Added a static PlaybackErrorMessage property to AudioFactory and a public const, kDefaultPlaybackErrorMessage, that will be used as the default message if the client does not set PlaybackErrorMessage. +- [SIL.Windows.Forms] Added KeysExtensions class with the IsNavigationKey extension method. ### Fixed - [SIL.DictionaryServices] Fix memory leak in LiftWriter @@ -71,7 +72,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Core] Support for RobustFile.GetAccessControl in all builds ### Fixed - - [SIL.Windows.Forms] Made ContributorsListControl.GetCurrentContribution() return null in the case when a valid row is not selected. - [SIL.WritingSystems] Make the English names for Chinese (Simplified) and Chinese (Traditional) consistent regardless of differences in Windows CultureInfo diff --git a/Palaso.sln.DotSettings b/Palaso.sln.DotSettings index b35dda771..b99101df8 100644 --- a/Palaso.sln.DotSettings +++ b/Palaso.sln.DotSettings @@ -6,6 +6,7 @@ GUI GUID IMDI + IME ISO LDML LIFT diff --git a/SIL.Windows.Forms.Keyboarding/Linux/IbusKeyboardSwitchingAdaptor.cs b/SIL.Windows.Forms.Keyboarding/Linux/IbusKeyboardSwitchingAdaptor.cs index 7ceba8146..1e89eaec0 100644 --- a/SIL.Windows.Forms.Keyboarding/Linux/IbusKeyboardSwitchingAdaptor.cs +++ b/SIL.Windows.Forms.Keyboarding/Linux/IbusKeyboardSwitchingAdaptor.cs @@ -100,7 +100,7 @@ protected virtual void SelectKeyboard(KeyboardDescription keyboard) context.SetEngine(ibusKeyboard.IBusKeyboardEngine.LongName); } - private void SetImePreeditWindowLocationAndSize(Control control) + private void SetImePreEditWindowLocationAndSize(Control control) { var eventHandler = GetEventHandlerForControl(control); if (eventHandler == null) @@ -165,8 +165,7 @@ private void OnControlRegistered(object sender, RegisterEventArgs e) e.Control.PreviewKeyDown += HandlePreviewKeyDown; e.Control.KeyPress += HandleKeyPress; - var scrollableControl = e.Control as ScrollableControl; - if (scrollableControl != null) + if (e.Control is ScrollableControl scrollableControl) scrollableControl.Scroll += HandleScroll; } @@ -182,8 +181,7 @@ private void OnControlRemoving(object sender, ControlEventArgs e) e.Control.KeyPress -= HandleKeyPress; e.Control.KeyDown -= HandleKeyDownAfterIbusHandledKey; - var scrollableControl = e.Control as ScrollableControl; - if (scrollableControl != null) + if (e.Control is ScrollableControl scrollableControl) scrollableControl.Scroll -= HandleScroll; var eventHandler = GetEventHandlerForControl(e.Control); @@ -202,7 +200,7 @@ private void OnControlRemoving(object sender, ControlEventArgs e) /// /// Passes the key event to ibus. This method deals with the special keys (Cursor up/down, - /// backspace etc) that usually shouldn't cause a commit. + /// backspace, etc.) that usually shouldn't cause a commit. /// private bool PassSpecialKeyEventToIbus(Control control, Keys keyChar, Keys modifierKeys) { @@ -212,8 +210,11 @@ private bool PassSpecialKeyEventToIbus(Control control, Keys keyChar, Keys modif private bool PassKeyEventToIbus(Control control, char keyChar, Keys modifierKeys) { - if (keyChar == 0x7f) // we get this for Ctrl-Backspace - keyChar = '\b'; + const char asciiDel = (char)0x7F; + const char backspace = '\b'; + + if (keyChar == asciiDel) // Ctrl+Backspace arrives as DEL; normalize to Backspace + keyChar = backspace; return PassKeyEventToIbus(control, keyChar, modifierKeys, true); } @@ -235,7 +236,7 @@ private bool PassKeyEventToIbus(Control control, int keySym, Keys modifierKeys, if (resetIfUnhandled) { - // If ProcessKeyEvent doesn't consume the key, we need to kill any preedits and + // If ProcessKeyEvent doesn't consume the key, we need to kill any pre-edits and // sync before continuing processing the keypress. We return false so that the // control can process the character. ResetAndWaitForCommit(control); @@ -254,8 +255,7 @@ private void HandleGotFocus(object sender, EventArgs e) // and the other time because we have to call the original window proc. However, only // the second time will the control report as being focused (or when we not intercept // the message then the first time) (see SimpleRootSite.OriginalWndProc). - var control = sender as Control; - if (control == null || !control.Focused) + if (!(sender is Control control) || !control.Focused) return; _ibusComm.FocusIn(); @@ -276,63 +276,94 @@ private void HandleLostFocus(object sender, EventArgs e) /// /// Inform input bus of Keydown events - /// This is useful to get warning of key that should stop the preedit + /// This is useful to get warning of key that should stop the pre-edit /// private void HandlePreviewKeyDown(object sender, PreviewKeyDownEventArgs e) { if (!_ibusComm.Connected) return; - var control = sender as Control; + var control = (Control)sender; var eventHandler = GetEventHandlerForControl(control); if (eventHandler == null) return; if (_needIMELocation) { - SetImePreeditWindowLocationAndSize(control); + SetImePreEditWindowLocationAndSize(control); _needIMELocation = false; } var key = e.KeyCode; - switch (key) + if (key == Keys.Escape) { - case Keys.Escape: - // These should end a preedit, so wait until that has happened - // before allowing the key to be processed. - ResetAndWaitForCommit(control); - return; - case Keys.Up: - case Keys.Down: - case Keys.Left: - case Keys.Right: - case Keys.Delete: - case Keys.PageUp: - case Keys.PageDown: - case Keys.Home: - case Keys.End: - case Keys.Back: - if (PassSpecialKeyEventToIbus(control, key, e.Modifiers)) - { - // If IBus handled the key we don't want the control to get it. However, - // we can't do this in PreviewKeyDown, so we temporarily subscribe to - // KeyDown and suppress the key event there. - control.KeyDown += HandleKeyDownAfterIbusHandledKey; - } - return; + // These should end a pre-edit, so wait until that has happened + // before allowing the key to be processed. + ResetAndWaitForCommit(control); + return; + } + + if (IsKeyHandledByIbus(key)) + { + if (PassSpecialKeyEventToIbus(control, key, e.Modifiers)) + { + // If IBus handled the key we don't want the control to get it. However, + // we can't do this in PreviewKeyDown, so we temporarily subscribe to + // KeyDown and suppress the key event there. + control.KeyDown += HandleKeyDownAfterIbusHandledKey; + } + return; } - // pass function keys onto ibus since they don't appear (on mono at least) as WM_SYSCHAR + + // pass function keys on to IBus since they don't appear (on Mono at least) as WM_SYSCHAR if (key >= Keys.F1 && key <= Keys.F24) PassSpecialKeyEventToIbus(control, key, e.Modifiers); } /// - /// Handles a key down. While a preedit is active we don't want the control to handle + /// Handles a key down. While a pre-edit is active we don't want the control to handle /// any of the keys that IBus deals with. /// private void HandleKeyDownAfterIbusHandledKey(object sender, KeyEventArgs e) { - switch (e.KeyCode) + if (IsKeyHandledByIbus(e.KeyCode) && sender is Control control) + { + var eventHandler = GetEventHandlerForControl(control); + if (eventHandler != null) + e.SuppressKeyPress = eventHandler.IsPreeditActive; + control.KeyDown -= HandleKeyDownAfterIbusHandledKey; + } + } + + /// + /// Gets whether the given key is expected to be handled by IBus when a pre-edit is active. + /// + /// + /// REVIEW: During pre-edit, IBus presumably handles both basic and modified navigation + /// keys like Ctrl+Left. Multiple AI sources agree this is true, but it has not been + /// verified in an actual Linux/IBus environment. Eberhard has proposed the following + /// testing procedure on Linux: + /// + /// Install Ubuntu 24.04 in a VM. + /// Install an IME that uses pre-edit: in a terminal run + /// sudo apt update && sudo apt install ibus-pinyin + /// Open Keyboard settings, click "+ Add Input Source", search for Pinyin by clicking + /// on the three dots below English. If you type the search term "pinyin" you'll get + /// "Other". Clicking that shows "Chinese (Pinyin)" which you then can add. + /// Close the settings. + /// In the terminal run ibus restart (or reboot). + /// The language/keyboard picker in the top bar should now offer "Chinese (Pinyin)". + /// + /// Open a text field in a SIL application that runs on Linux and uses + /// `KeyboardController` (maybe WeSay), switch to Chinese (Pinyin), and begin typing to + /// enter pre-edit mode. Then press modified navigation keys like Ctrl+Left and verify + /// that IBus handles them correctly (e.g., the pre-edit is committed or cancelled as + /// expected, rather than the key being ignored or causing duplicate input). + /// + /// + private bool IsKeyHandledByIbus(Keys key) + { + switch (key & Keys.KeyCode) { case Keys.Up: case Keys.Down: @@ -344,13 +375,10 @@ private void HandleKeyDownAfterIbusHandledKey(object sender, KeyEventArgs e) case Keys.Home: case Keys.End: case Keys.Back: - var control = sender as Control; - var eventHandler = GetEventHandlerForControl(control); - if (eventHandler != null) - e.SuppressKeyPress = eventHandler.IsPreeditActive; - control.KeyDown -= HandleKeyDownAfterIbusHandledKey; - break; + return true; } + + return false; } /// @@ -360,17 +388,20 @@ private void HandleKeyDownAfterIbusHandledKey(object sender, KeyEventArgs e) /// this method gets called. We forward the key press to IBus. If IBus swallowed the key /// it will return true, so no further handling is done by the control, otherwise the /// control will process the key and update the selection. - /// If IBus swallows the key event, it will either raise the UpdatePreeditText event, - /// allowing the event handler to insert the composition as preedit (and update the - /// selection), or it will raise the CommitText event so that the event handler can - /// remove the preedit, replace it with the final composition string and update the - /// selection. Some IBus keyboards might raise a ForwardKeyEvent (handled by - /// ) prior to calling CommitText to - /// simulate a key press (e.g. backspace) so that the event handler can modify the - /// existing text of the control. + /// If IBus swallows the key event, it will either raise the + /// event, allowing the event handler to + /// insert the composition as pre-edit (and update the selection), or it will raise the + /// event so that the event handler can + /// remove the pre-edit, replace it with the final composition string and update the + /// selection. Some IBus keyboards might raise a + /// (handled by + /// ) prior to raising + /// to simulate a key press (e.g. backspace) + /// so that the event handler can modify the existing text of the control. /// IBus might also open a pop-up window at the location we told it /// () to display possible - /// compositions. However, it will still call UpdatePreeditText. + /// compositions. However, it will still raise + /// . private void HandleKeyPress(object sender, KeyPressEventArgs e) { e.Handled = PassKeyEventToIbus(sender as Control, e.KeyChar, Control.ModifierKeys); @@ -390,7 +421,7 @@ private void HandleScroll(object sender, ScrollEventArgs e) if (!_ibusComm.Connected) return; - SetImePreeditWindowLocationAndSize(sender as Control); + SetImePreEditWindowLocationAndSize(sender as Control); } #endregion diff --git a/SIL.Windows.Forms/Extensions/KeysExtensions.cs b/SIL.Windows.Forms/Extensions/KeysExtensions.cs new file mode 100644 index 000000000..f89ee4caf --- /dev/null +++ b/SIL.Windows.Forms/Extensions/KeysExtensions.cs @@ -0,0 +1,39 @@ +using System.Windows.Forms; + +namespace SIL.Windows.Forms.Extensions +{ + public static class KeysExtensions + { + /// + /// Determines whether the specified key is considered a navigation key. + /// + /// The key (or key-combination) to examine. + /// If true, the letter Keys.A will be treated as a + /// navigation key (even if the value does not explicitly include + /// Keys.Control). In contexts where Ctrl-A is not a shortcut for Select All or that + /// behavior is not relevant, pass false or omit this optional parameter. + /// If represents the combination Ctrl-A (i.e., + /// Keys.Control | Keys.A), but is false, + /// this method will return false. + public static bool IsNavigationKey(this Keys key, bool ctrlPressed = false) + { + switch (key & Keys.KeyCode) + { + case Keys.Left: + case Keys.Up: + case Keys.Down: + case Keys.Right: + case Keys.Home: + case Keys.End: + case Keys.PageDown: + case Keys.PageUp: + return true; + case Keys.A: + return ctrlPressed; + default: + return false; + } + } + } + +} diff --git a/SIL.Windows.Forms/Widgets/BetterGrid/CalendarEditingControl.cs b/SIL.Windows.Forms/Widgets/BetterGrid/CalendarEditingControl.cs index 2469217c1..8818bc85d 100644 --- a/SIL.Windows.Forms/Widgets/BetterGrid/CalendarEditingControl.cs +++ b/SIL.Windows.Forms/Widgets/BetterGrid/CalendarEditingControl.cs @@ -1,6 +1,7 @@ -using System; +using System; using System.Drawing; using System.Windows.Forms; +using SIL.Windows.Forms.Extensions; namespace SIL.Windows.Forms.Widgets.BetterGrid { @@ -114,21 +115,8 @@ public void ApplyCellStyleToEditingControl(DataGridViewCellStyle dataGridViewCel /// ------------------------------------------------------------------------------------ public bool EditingControlWantsInputKey(Keys key, bool dataGridViewWantsInputKey) { - // Let the DateTimePicker handle the keys listed. - switch (key & Keys.KeyCode) - { - case Keys.Left: - case Keys.Up: - case Keys.Down: - case Keys.Right: - case Keys.Home: - case Keys.End: - case Keys.PageDown: - case Keys.PageUp: - return true; - default: - return !dataGridViewWantsInputKey; - } + // Let the DateTimePicker handle navigation keys. + return key.IsNavigationKey() || !dataGridViewWantsInputKey; } /// ------------------------------------------------------------------------------------ @@ -138,10 +126,7 @@ public void PrepareEditingControlForEdit(bool selectAll) } /// ------------------------------------------------------------------------------------ - public bool RepositionEditingControlOnValueChange - { - get { return false; } - } + public bool RepositionEditingControlOnValueChange => false; #endregion }