Remove Timer from WindowsKeyboardSwitchingAdapter and add IME diag#1495
Remove Timer from WindowsKeyboardSwitchingAdapter and add IME diag#1495jasonleenaylor wants to merge 1 commit intomasterfrom
Conversation
…tics Replace the Timer-based deferred IME conversion status restore with synchronous restore. The Timer caused jittery, unreliable IME switching for Chinese Pinyin and other TSF-based IMEs. The synchronous approach ensures ImmSetConversionStatus is applied before any subsequent SaveImeConversionStatus can capture stale values. Add Trace.WriteLine diagnostic logging throughout keyboard switching, IME save/restore, and post-switch state verification to aid debugging of intermittent IME composition issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| /// This class handles switching for normal Windows keyboards, Windows IME keyboards, and Keyman 10 keyboards | ||
| /// </summary> | ||
| internal class WindowsKeyboardSwitchingAdapter : IKeyboardSwitchingAdaptor, IDisposable | ||
| internal class WindowsKeyboardSwitchingAdapter : IKeyboardSwitchingAdaptor |
There was a problem hiding this comment.
🔴 Missing CHANGELOG.md update required by AGENTS.md
The AGENTS.md rule file mandates: "If the suggested code changes functionality, fixes a bug, or adds a feature, you must generate an update for CHANGELOG.md." This PR clearly changes functionality — it removes the Timer-based IME conversion status retry mechanism from WindowsKeyboardSwitchingAdapter, removes IDisposable from the class, and adds diagnostic tracing plus new Win32 P/Invoke declarations (ImmGetOpenStatus, ImmSetOpenStatus, GetClassName, GetKeyboardLayout). However, CHANGELOG.md was not updated with any entry under the [Unreleased] section. The ### Changed subsection should describe the removal of the timer-based workaround and behavioral change to IME switching.
Prompt for agents
Add a CHANGELOG.md entry under the ## [Unreleased] section. Under the ### Changed subsection (create it if it doesn't exist at the top of [Unreleased]), add an entry such as:
- **[SIL.Windows.Forms.Keyboarding]** Removed Timer-based IME conversion status retry from WindowsKeyboardSwitchingAdapter; added diagnostic tracing for IME keyboard switching.
The entry should go in CHANGELOG.md under the ## [Unreleased] section.
Was this helpful? React with 👍 or 👎 to provide feedback.
| RestoreImeConversionStatus(keyboard); | ||
| TraceImeState("PostSwitch", focusAfter, keyboard); |
There was a problem hiding this comment.
🚩 Removal of Timer-based IME retry may regress intermittent IME activation issues
The old code had a Timer (500ms interval) that would call RestoreImeConversionStatus a second time after keyboard switching, with the comment "Restore it even though sometimes windows will ignore us" and "Start the timer for restoring IME status for when windows ignores us." This was a deliberate workaround for an intermittent Windows behavior where the OS would silently reset IME conversion status shortly after it was set. The new code calls RestoreImeConversionStatus only once (line 67) and relies on the added diagnostic tracing to investigate the root cause instead. This is a reasonable approach for diagnosis, but the behavioral change means IME users (particularly Chinese/Japanese/Korean input) may see the intermittent mode-reset issue return until a more robust fix is implemented.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private bool SwitchKeyboard(KeyboardDescription winKeyboard) | ||
| { | ||
| var keyboard = winKeyboard as WinKeyboardDescription; | ||
| if (IsSwitchingKeyboards) | ||
| return true; | ||
|
|
||
| IsSwitchingKeyboards = true; | ||
| try | ||
| { | ||
| if (keyboard?.InputLanguage?.Culture == null || keyboard.InputProcessorProfile.LangId == 0) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| _expectedKeyboard = keyboard; | ||
| return Platform.IsMono || SwitchByProfile(keyboard); | ||
| } | ||
| finally | ||
| if (keyboard?.InputLanguage?.Culture == null || keyboard.InputProcessorProfile.LangId == 0) | ||
| { | ||
| IsSwitchingKeyboards = false; | ||
| Trace.WriteLine($"[KbdSwitch] SwitchKeyboard: invalid keyboard description (culture={keyboard?.InputLanguage?.Culture}, langId=0x{keyboard?.InputProcessorProfile.LangId:X4})"); | ||
| return false; | ||
| } | ||
|
|
||
| return Platform.IsMono || SwitchByProfile(keyboard); | ||
| } |
There was a problem hiding this comment.
🚩 Removal of IsSwitchingKeyboards re-entrancy guard
The old code had a IsSwitchingKeyboards flag that prevented re-entrant calls to SwitchKeyboard. This was relevant because SwitchByProfile makes COM calls (ChangeCurrentLanguage, ActivateProfile) that could theoretically pump Windows messages on the UI thread, potentially triggering another keyboard activation. The new code removes this guard entirely. The ActivateKeyboard method at line 29 does check KeyboardController.Instance.ActiveKeyboard == keyboard as a short-circuit, but during switching ActiveKeyboard is set to NullKeyboard (see KeyboardDescription.cs:64), so this wouldn't prevent re-entrant activation of the same keyboard. In practice, re-entrancy through COM message pumping during these specific calls is unlikely but not impossible.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Replace the Timer-based deferred IME conversion status restore with synchronous restore. The Timer caused jittery, unreliable IME switching for Chinese Pinyin and other TSF-based IMEs. The synchronous approach ensures ImmSetConversionStatus is applied before any subsequent SaveImeConversionStatus can capture stale values.
Add Trace.WriteLine diagnostic logging throughout keyboard switching, IME save/restore, and post-switch state verification to aid debugging of intermittent IME composition issues.
This change is