Remove Timer from WindowsKeyboardSwitchingAdapter and add IME diag

CHANGELOG.md

@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [SIL.Windows.Forms] Added KeysExtensions class with the IsNavigationKey extension method.
 
 ### Fixed
+- [SIL.Windows.Forms.Keyboarding] Removed Timer-based deferred IME conversion status restore from WindowsKeyboardSwitchingAdapter, which disrupted active Chinese Pinyin IME compositions (LT-22442). Added diagnostic tracing for keyboard switching and IME state.
 - [SIL.DictionaryServices] Fix memory leak in LiftWriter
 - [SIL.WritingSystems] Fix IetfLanguageTag.GetGeneralCode to handle cases when zh-CN or zh-TW is a prefix and not the whole string.
 - [SIL.WritingSystems] More fixes to consistently use 繁体中文 and 简体中文 for Traditional and Simplified Chinese native language names, and Chinese (Traditional) and Chinese (Simplified) for their English names.

SIL.Windows.Forms.Keyboarding/Windows/Win32.cs

@@ -118,6 +118,25 @@ public static extern bool ImmSetConversionStatus(HandleRef context, int conversi
 
 		[DllImport("imm32.dll", CharSet = CharSet.Unicode)]
 		public static extern int ImmGetDescription(IntPtr hkl, [MarshalAs(UnmanagedType.LPTStr)] StringBuilder description, uint bufLen);
+
+		/// <summary>
+		/// Determines the open or close status of the IME.
+		/// </summary>
+		/// <param name="hIMC">Handle to the input context.</param>
+		/// <returns>Returns true if the IME is open, or false otherwise.</returns>
+		[DllImport("imm32.dll", CharSet = CharSet.Auto)]
+		[return: MarshalAs(UnmanagedType.Bool)]
+		public static extern bool ImmGetOpenStatus(HandleRef hIMC);
+
+		/// <summary>
+		/// Opens or closes the IME.
+		/// </summary>
+		/// <param name="hIMC">Handle to the input context.</param>
+		/// <param name="open">true to open the IME, or false to close it.</param>
+		/// <returns>Returns true if successful, or false otherwise.</returns>
+		[DllImport("imm32.dll", CharSet = CharSet.Auto)]
+		[return: MarshalAs(UnmanagedType.Bool)]
+		public static extern bool ImmSetOpenStatus(HandleRef hIMC, bool open);
 		#endregion
 
 		#region user32.dll
@@ -148,6 +167,18 @@ public static IntPtr GetFocus()
 
 		[DllImport("user32.dll")]
 		public static extern int GetKeyboardLayoutList(int nBuff, [Out] IntPtr lpList);
+
+		/// <summary>
+		/// Retrieves the name of the class to which the specified window belongs.
+		/// </summary>
+		[DllImport("user32.dll", CharSet = CharSet.Auto)]
+		public static extern int GetClassName(IntPtr hWnd, StringBuilder lpClassName, int nMaxCount);
+
+		/// <summary>
+		/// Retrieves the keyboard layout for the specified thread.
+		/// </summary>
+		[DllImport("user32.dll")]
+		public static extern IntPtr GetKeyboardLayout(uint idThread);
 		#endregion
 
 		#region shlwapi.dll

SIL.Windows.Forms.Keyboarding/Windows/WindowsKeyboardSwitchingAdapter.cs

@@ -5,97 +5,120 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Runtime.InteropServices;
+using System.Text;
 using System.Windows.Forms;
 using SIL.Keyboarding;
 using SIL.PlatformUtilities;
-using Timer = System.Windows.Forms.Timer;
 
 namespace SIL.Windows.Forms.Keyboarding.Windows
 {
 	/// <summary>
 	/// 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
 	{
-		#region Variables used for windows IME Mode switching hack
-		public Timer Timer { get; private set; }
-		private KeyboardDescription _expectedKeyboard;
 		private WinKeyboardAdaptor _adaptor;
-		private bool HasSwitchedLanguages { get; set; }
-		public bool IsSwitchingKeyboards { get; set; }
-		#endregion
+		private bool _isSwitchingKeyboards;
 
 		public WindowsKeyboardSwitchingAdapter(WinKeyboardAdaptor adaptor)
 		{
 			_adaptor = adaptor;
-			Timer = new Timer { Interval = 500 };
-			Timer.Tick += OnTimerTick;
-			Timer.Enabled = true;
-		}
-
-		private void OnTimerTick(object sender, EventArgs eventArgs)
-		{
-			if (_expectedKeyboard == null || HasSwitchedLanguages)
-			{
-				return;
-			}
-			RestoreImeConversionStatus(_expectedKeyboard);
-			IsSwitchingKeyboards = false;
-			Timer.Stop();
 		}
 
 		public bool ActivateKeyboard(KeyboardDescription keyboard)
 		{
 			if (KeyboardController.Instance.ActiveKeyboard == keyboard)
+			{
+				Trace.WriteLine($"[KbdSwitch] ActivateKeyboard: already active, skipping: {keyboard}");
 				return true;
-			Timer.Start();
+			}
+			Trace.WriteLine($"[KbdSwitch] ActivateKeyboard: switching from {KeyboardController.Instance.ActiveKeyboard} to {keyboard}");
 			return SwitchKeyboard(keyboard);
 		}
 
 		private bool SwitchKeyboard(KeyboardDescription winKeyboard)
 		{
 			var keyboard = winKeyboard as WinKeyboardDescription;
-			if (IsSwitchingKeyboards)
+			if (_isSwitchingKeyboards)
+			{
+				Trace.WriteLine($"[KbdSwitch] SwitchKeyboard: reentrant call blocked for {keyboard?.Name}");
 				return true;
+			}
 
-			IsSwitchingKeyboards = true;
-			try
+			if (keyboard?.InputLanguage?.Culture == null || keyboard.InputProcessorProfile.LangId == 0)
 			{
-				if (keyboard?.InputLanguage?.Culture == null || keyboard.InputProcessorProfile.LangId == 0)
-				{
-					return false;
-				}
+				Trace.WriteLine($"[KbdSwitch] SwitchKeyboard: invalid keyboard description (culture={keyboard?.InputLanguage?.Culture}, langId=0x{keyboard?.InputProcessorProfile.LangId:X4})");
+				return false;
+			}
 
-				_expectedKeyboard = keyboard;
+			_isSwitchingKeyboards = true;
+			try
+			{
 				return Platform.IsMono || SwitchByProfile(keyboard);
 			}
 			finally
 			{
-				IsSwitchingKeyboards = false;
+				_isSwitchingKeyboards = false;
 			}
 		}
 
 		private bool SwitchByProfile(WinKeyboardDescription keyboard)
 		{
+			var focusBefore = Win32.GetFocus();
+			Trace.WriteLine($"[KbdSwitch] SwitchByProfile START: {keyboard.Name}, LangId=0x{keyboard.InputProcessorProfile.LangId:X4}, ProfileType={keyboard.InputProcessorProfile.ProfileType}, focus=0x{focusBefore:X}, focusClass={GetWindowClassName(focusBefore)}");
+
 			_adaptor.ProcessorProfiles.ChangeCurrentLanguage(keyboard.InputProcessorProfile.LangId);
+			Trace.WriteLine($"[KbdSwitch] ChangeCurrentLanguage done, CurrentInputLanguage={InputLanguage.CurrentInputLanguage.Culture.Name}");
 
 			Guid classId = keyboard.InputProcessorProfile.ClsId;
 			Guid guidProfile = keyboard.InputProcessorProfile.GuidProfile;
 			_adaptor.ProfileManager.ActivateProfile(keyboard.InputProcessorProfile.ProfileType, keyboard.InputProcessorProfile.LangId, ref classId, ref guidProfile,
 				keyboard.InputProcessorProfile.Hkl, TfIppMf.ForProcess);
 
-			RestoreImeConversionStatus(
-				keyboard); // Restore it even though sometimes windows will ignore us
-			Timer.Stop();
-			Timer.Start(); // Start the timer for restoring IME status for when windows ignores us.
+			var focusAfter = Win32.GetFocus();
+			var hkl = Win32.GetKeyboardLayout(0);
+			Trace.WriteLine($"[KbdSwitch] ActivateProfile done, CurrentInputLanguage={InputLanguage.CurrentInputLanguage.Culture.Name}, focus=0x{focusAfter:X} (focusChanged={focusBefore != focusAfter}), threadHKL=0x{hkl:X}");
 
+			RestoreImeConversionStatus(keyboard);
+			TraceImeState("PostSwitch", focusAfter, keyboard);
+
+			Trace.WriteLine("[KbdSwitch] SwitchByProfile END");
 			return true;
 		}
 
+		/// <summary>
+		/// Log detailed IME state for diagnosing intermittent IME activation issues.
+		/// </summary>
+		private void TraceImeState(string context, IntPtr focusHwnd, WinKeyboardDescription keyboard)
+		{
+			var windowHandle = new HandleRef(this, focusHwnd);
+			var contextPtr = Win32.ImmGetContext(windowHandle);
+			if (contextPtr == IntPtr.Zero)
+			{
+				Trace.WriteLine($"[KbdSwitch] {context}: ImmGetContext=NULL (focus=0x{focusHwnd:X}) — no IME context available");
+				return;
+			}
+			var contextHandle = new HandleRef(this, contextPtr);
+			var isOpen = Win32.ImmGetOpenStatus(contextHandle);
+			int convMode, sentMode;
+			Win32.ImmGetConversionStatus(contextHandle, out convMode, out sentMode);
+			Win32.ImmReleaseContext(windowHandle, contextHandle);
+			Trace.WriteLine($"[KbdSwitch] {context}: IME open={isOpen}, conversion=0x{convMode:X}, sentence=0x{sentMode:X}, focus=0x{focusHwnd:X}, focusClass={GetWindowClassName(focusHwnd)}");
+		}
+
+		private static string GetWindowClassName(IntPtr hwnd)
+		{
+			if (hwnd == IntPtr.Zero)
+				return "(null)";
+			var sb = new StringBuilder(256);
+			Win32.GetClassName(hwnd, sb, sb.Capacity);
+			return sb.ToString();
+		}
+
 
 		public void DeactivateKeyboard(KeyboardDescription keyboard)
 		{
-			Timer.Stop();
+			Trace.WriteLine($"[KbdSwitch] DeactivateKeyboard: {keyboard}");
 			SaveImeConversionStatus((WinKeyboardDescription)keyboard);
 		}
 
@@ -106,22 +129,30 @@ public void DeactivateKeyboard(KeyboardDescription keyboard)
 		private void SaveImeConversionStatus(WinKeyboardDescription winKeyboard)
 		{
 			if (winKeyboard == null)
+			{
+				Trace.WriteLine("[KbdSwitch] SaveIME: keyboard is null, skipping");
 				return;
+			}
 
 			if (InputLanguage.CurrentInputLanguage.Culture.Name != winKeyboard.InputLanguage.Culture.Name)
 			{
-				// Users can switch the keyboards without switching fields, don't save unrelated IME status
+				Trace.WriteLine($"[KbdSwitch] SaveIME: culture mismatch (current={InputLanguage.CurrentInputLanguage.Culture.Name}, keyboard={winKeyboard.InputLanguage.Culture.Name}), skipping");
 				return;
 			}
 
-			var windowHandle = new HandleRef(this, Win32.GetFocus());
+			var focusHwnd = Win32.GetFocus();
+			var windowHandle = new HandleRef(this, focusHwnd);
 			var contextPtr = Win32.ImmGetContext(windowHandle);
 			if (contextPtr == IntPtr.Zero)
+			{
+				Trace.WriteLine($"[KbdSwitch] SaveIME: ImmGetContext returned NULL (focus=0x{focusHwnd:X}), skipping");
 				return;
+			}
 			var contextHandle = new HandleRef(this, contextPtr);
 			int conversionMode;
 			int sentenceMode;
 			Win32.ImmGetConversionStatus(contextHandle, out conversionMode, out sentenceMode);
+			Trace.WriteLine($"[KbdSwitch] SaveIME: {winKeyboard.Name} conversion=0x{conversionMode:X}, sentence=0x{sentenceMode:X}");
 			winKeyboard.ConversionMode = conversionMode;
 			winKeyboard.SentenceMode = sentenceMode;
 			Win32.ImmReleaseContext(windowHandle, contextHandle);
@@ -141,13 +172,17 @@ private void RestoreImeConversionStatus(IKeyboardDefinition keyboard)
 			// Restore the state of the new keyboard to the previous value. If we don't do
 			// that e.g. in Chinese IME the input mode will toggle between English and
 			// Chinese (LT-7487 et al).
-			var windowHandle = new HandleRef(this, Win32.GetFocus());
+			var focusHwnd = Win32.GetFocus();
+			var windowHandle = new HandleRef(this, focusHwnd);
 
 			// NOTE: Windows uses the same context for all windows of the current thread, so it
 			// doesn't really matter which window handle we pass.
 			var contextPtr = Win32.ImmGetContext(windowHandle);
 			if (contextPtr == IntPtr.Zero)
+			{
+				Trace.WriteLine($"[KbdSwitch] RestoreIME: ImmGetContext returned NULL (focus=0x{focusHwnd:X}), skipping");
 				return;
+			}
 
 			// NOTE: Chinese Pinyin IME allows to switch between Chinese and Western punctuation.
 			// This can be selected in both Native and Alphanumeric conversion mode. However,
@@ -159,7 +194,12 @@ private void RestoreImeConversionStatus(IKeyboardDefinition keyboard)
 			Win32.ImmGetConversionStatus(contextHandle, out currentConversionMode, out currentSentenceMode);
 			if (winKeyboard.ConversionMode != currentConversionMode || winKeyboard.SentenceMode != currentSentenceMode)
 			{
-				Win32.ImmSetConversionStatus(contextHandle, winKeyboard.ConversionMode, winKeyboard.SentenceMode);
+				var success = Win32.ImmSetConversionStatus(contextHandle, winKeyboard.ConversionMode, winKeyboard.SentenceMode);
+				Trace.WriteLine($"[KbdSwitch] RestoreIME: {winKeyboard.Name} set conversion 0x{currentConversionMode:X}->0x{winKeyboard.ConversionMode:X}, sentence 0x{currentSentenceMode:X}->0x{winKeyboard.SentenceMode:X}, success={success}");
+			}
+			else
+			{
+				Trace.WriteLine($"[KbdSwitch] RestoreIME: {winKeyboard.Name} modes already match (conversion=0x{currentConversionMode:X}, sentence=0x{currentSentenceMode:X})");
 			}
 			Win32.ImmReleaseContext(windowHandle, contextHandle);
 		}
@@ -178,30 +218,5 @@ public KeyboardDescription ActiveKeyboard
 			}
 		}
 
-		~WindowsKeyboardSwitchingAdapter()
-		{
-			Dispose(false);
-			// The base class finalizer is called automatically.
-		}
-
-		/// <summary/>
-		/// <remarks>Must not be virtual.</remarks>
-		public void Dispose()
-		{
-			Dispose(true);
-			// This object will be cleaned up by the Dispose method.
-			// Therefore, you should call GC.SuppressFinalize to
-			// take this object off the finalization queue
-			// and prevent finalization code for this object
-			// from executing a second time.
-			GC.SuppressFinalize(this);
-		}
-
-		protected virtual void Dispose(bool disposing)
-		{
-			Debug.WriteLineIf(!disposing, "****************** " + GetType().Name + " 'disposing' is false. ******************");
-			Timer?.Dispose();
-			Timer = null;
-		}
 	}
 }