fix(mac): apply OSK modifiers to typing on physical keyboard

Fixes: #12584

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.m

@@ -944,6 +944,7 @@ - (void)registerConfigurationWindow:(NSWindowController *)window {
 }
 
 - (void)showOSK {
+  [self.oskWindow prepareToShowOsk];
   [[self.oskWindow window] makeKeyAndOrderFront:nil];
   [[self.oskWindow window] setLevel:NSStatusWindowLevel];
   [[self.oskWindow window] setTitle:self.oskWindowTitle];
@@ -1242,13 +1243,32 @@ - (NSEventModifierFlags) determineModifiers {
   NSEventModifierFlags modifierFlags = 0;
 
   if (self.receivedKeyDownFromOsk) {
+    /**
+     * the event was generated from the OSK, so use the oskEventModifiers in effect at time of event generation
+     */
     modifierFlags = self.oskEventModifiers;
     os_log_debug([KMLogs eventsLog], "--- use modifiers from OSK, oskEventModifiers: 0x%lX", (unsigned long)modifierFlags);
     self.oskEventModifiers = 0;
-  } else {
+  } else { 
+    /**
+     * the event originated from the physical keyboard, use the modifiers on kCGEventFlagsChanged events
+     */
     NSEventModifierFlags originalModifiers = self.currentModifiers;
     modifierFlags = [self.modifierMapping adjustModifiers:originalModifiers];
     os_log_debug([KMLogs eventsLog], "--- use adjusted modifiers from current state: 0x%lX", (unsigned long)modifierFlags);
+
+    /**
+     * If the OSK is open, then we also need to apply its modifiers, if any, to this event originating from the physical keyboard.
+     * If the OSK is closed, its modifiers will be zero.
+     */
+    NSEventModifierFlags oskModifiers = [self.oskWindow getOskEventModifierFlags];
+    if (oskModifiers != 0) {
+      os_log_debug([KMLogs eventsLog], "--- modifiers from OSK to apply to physical keyboard event: 0x%lX", (unsigned long)oskModifiers);
+
+      // combine osk modifiers with adjusted modifiers
+      modifierFlags = oskModifiers | modifierFlags;
+      os_log_debug([KMLogs eventsLog], "--- combined modifiers to apply to physical keyboard event: 0x%lX", (unsigned long)modifierFlags);
+    }
   }
 
   return modifierFlags;

mac/Keyman4MacIM/Keyman4MacIM/OnScreenKeyboard/OSKWindowController.h

@@ -13,6 +13,8 @@
 
 @property (nonatomic, weak) IBOutlet OSKView *oskView;
 
+- (void)prepareToShowOsk;
 - (void)resetOSK;
+- (NSEventModifierFlags)getOskEventModifierFlags;
 
 @end

mac/Keyman4MacIM/Keyman4MacIM/OnScreenKeyboard/OSKWindowController.m

@@ -66,20 +66,18 @@ - (void)windowDidResize:(NSNotification *)notification {
   [self.oskView resizeOSKLayout];
 }
 
+- (void)prepareToShowOsk {
+  os_log_info([KMLogs oskLog], "OSKWindowController prepareToShowOsk");
+}
+
 - (void)windowWillClose:(NSNotification *)notification {
   os_log_debug([KMLogs oskLog], "OSKWindowController windowWillClose");
   [KMSettingsRepository.shared writeShowOskOnActivate:NO];
 
-  // whenever the OSK is closed clear all of its modifier keys
+  // whenever the OSK is closing clear all of its modifier keys
   [self.oskView clearOskModifiers];
 }
 
-- (void) showWindow:(id) sender {
-  [super showWindow:sender];
-
-  os_log_info([KMLogs oskLog], "OSKWindowController showWindow");
-}
-
 - (void)helpAction:(id)sender {
   NSString *kvkPath = [self AppDelegate].kvk.filePath;
   if (!kvkPath)
@@ -106,6 +104,16 @@ - (void)resetOSK {
   }
 }
 
+/**
+ * returns current event flags representing the state of the modifiers on the OSK or zero if OSK is closed
+ */
+- (NSEventModifierFlags)getOskEventModifierFlags {
+  if (self.window.isVisible) {
+    return [self.oskView getOskEventModifierFlags];
+  } else {
+    return 0;
+  }
+}
 - (void)startTimerWithTimeInterval:(NSTimeInterval)interval {
   if (_modFlagsTimer == nil) {
     TimerTarget *timerTarget = [[TimerTarget alloc] init];

mac/KeymanEngine4Mac/KeymanEngine4Mac/KME/OnScreenKeyboard/OSKView.h

@@ -26,6 +26,7 @@
 - (void)setPhysicalControlState:(BOOL)ctrlState;
 - (void)setOskControlState:(BOOL)oskCtrlState;
 - (void)clearOskModifiers;
+- (NSEventModifierFlags)getOskEventModifierFlags;
 - (void)resetOSK;
 - (void)resizeOSKLayout;
 - (int64_t)createOskEventUserData;

mac/KeymanEngine4Mac/KeymanEngine4Mac/KME/OnScreenKeyboard/OSKView.m

@@ -140,7 +140,7 @@ - (void)setKvk:(KVKFile *)kvk {
   _oskLayout = nil;
   _oskDefaultNKeys = nil;
 
-  [self updateKeyLabelsForState];
+  [self updateKeyLabelsForCurrentLayer];
 }
 
 - (void)initOSKKeys {
@@ -174,7 +174,7 @@ - (void)initOSKKeys {
     py += keyHeight;
   }
 
-  [self updateKeyLabelsForState];
+  [self updateKeyLabelsForCurrentLayer];
 }
 
 - (NSArray *)oskLayout {
@@ -436,7 +436,7 @@ - (void)clearOskModifiers {
   _oskOptionState = NO;
   _oskControlState = NO;
   [self updateModifierKeysForState];
-  [self updateKeyLabelsForState];
+  [self updateKeyLabelsForCurrentLayer];
 }
 
 - (void)resetOSK {
@@ -487,6 +487,47 @@ - (void)keyAction:(id)sender {
   }
 }
 
+/**
+ * get the event modifier flags representing the current state of the osk modifiers
+ */
+- (NSEventModifierFlags)getOskEventModifierFlags {
+  return [self getEventModifierFlags:self.oskShiftState optionFlag:self.oskOptionState controlFlag:self.oskControlState];
+}
+
+/**
+ * get the event modifier flags that represent the combined state of the physical and osk modifiers
+ */
+- (NSEventModifierFlags) getCombinedEventModifierFlags {
+  BOOL shift = self.physicalShiftState | self.oskShiftState;
+  BOOL option = self.physicalOptionState | self.oskOptionState;
+  BOOL control = self.physicalControlState | self.oskControlState;
+
+  return [self getEventModifierFlags:shift optionFlag:option controlFlag:control];
+}
+
+/**
+ * get the NSEventModifierFlags value that corresponds to the specified modifier states
+ */
+- (NSEventModifierFlags)getEventModifierFlags:(BOOL)shift optionFlag:(BOOL)option controlFlag:(BOOL)control {
+  NSEventModifierFlags modifierFlags = 0;
+
+  if (shift) {
+    modifierFlags = modifierFlags | NSEventModifierFlagShift;
+  }
+  /**
+   * Both left and right option keys on the OSK cause oskOptionState to be true without distinction of left or right.
+   * However, the generated event is a right alt so that it will trigger the right alt rules in the Keyman keyboard.
+   */
+  if (option) {
+    modifierFlags = modifierFlags | MK_RIGHT_ALT_MASK;
+  }
+  if (control) {
+    modifierFlags = modifierFlags | NSEventModifierFlagControl;
+  }
+
+  return modifierFlags;
+}
+
 /**
  * Create the 64-bit value to pass in the kCGEventSourceUserData field of the generated CGEvent
  * The upper-most bit is set to distinguish it from the default value of zero, and the lower 32 bits
@@ -500,22 +541,9 @@ - (void)keyAction:(id)sender {
 - (int64_t) createOskEventUserData {
   // set bit to identify this user data as originating from the OSK
   int64_t oskEventData = OSK_EVENT_FLAG;
-
-  if (self.oskShiftState || self.physicalShiftState) {
-    oskEventData = oskEventData | kCGEventFlagMaskShift;
-  }
-  /**
-   * Both left and right option keys on the OSK cause oskOptionState to be true without distinction of left or right.
-   * However, the generated event is a right alt so that it will trigger the right alt rules in the Keyman keyboard.
-   */
-  if (self.oskOptionState || self.physicalOptionState) {
-    oskEventData = oskEventData | MK_RIGHT_ALT_MASK;
-  }
-  if (self.oskControlState || self.oskControlState) {
-    oskEventData = oskEventData | kCGEventFlagMaskControl;
-  }
-
-  return oskEventData;
+  NSEventModifierFlags eventModifierFlags = [self getCombinedEventModifierFlags];
+
+  return oskEventData | eventModifierFlags;
 }
 
 - (void)handleKeyEvent:(NSEvent *)event {
@@ -536,13 +564,20 @@ - (void)setKeyPressedOff:(KeyView *)keyView {
 }
 
 /**
- * get the modifier flags that represent the combined state of the physical and osk modifiers
+ * get the Keyman modifier flags that represent the combined state of the physical and osk modifiers
  */
-- (WORD) getCombinedModifierFlags {
+- (WORD) getKeymanModifierFlagsForCurrentLayer {
   BOOL shift = self.physicalShiftState | self.oskShiftState;
   BOOL option = self.physicalOptionState | self.oskOptionState;
   BOOL control = self.physicalControlState | self.oskControlState;
-
+
+  return [self getKeymanModifierFlags:shift optionFlag:option controlFlag:control];
+}
+
+/**
+ * get Keyman modifier flags for the specified modifier states
+ */
+- (WORD)getKeymanModifierFlags:(BOOL)shift optionFlag:(BOOL)option controlFlag:(BOOL)control {
   WORD flags = 0;
   if (shift) {
     flags |= KVKS_SHIFT;
@@ -565,7 +600,7 @@ - (WORD) getCombinedModifierFlags {
  * Display the key labels that correspond to the current state of the modifier keys.
  * The layer shown depends on both the physical modifier state and the osk modifier state.
  */
-- (void)updateKeyLabelsForState {
+- (void)updateKeyLabelsForCurrentLayer {
   os_log_debug([KMELogs oskLog], "OSKView updateKeyLabelsForState, phsyical modifiers [shift: %d, option: %d, control: %d]\nosk modifiers [shift: %d, option: %d, control: %d]", self.physicalShiftState, self.physicalOptionState, self.physicalControlState, self.oskShiftState, self.oskOptionState, self.oskControlState);
 
   [self resetKeyLabels];
@@ -574,13 +609,13 @@ - (void)updateKeyLabelsForState {
   if (nkeys == nil)
     nkeys = self.oskDefaultNKeys;
 
-  WORD flags = [self getCombinedModifierFlags];
+  WORD keymanFlagsForCurrentLayer = [self getKeymanModifierFlagsForCurrentLayer];
 
   NSString *ansiFont = [self ansiFont];
   NSString *unicodeFont = [self unicodeFont];
   unsigned short keyCode;
   for (NKey *nkey in nkeys) {
-    if (nkey.modifierFlags == flags) {
+    if (nkey.modifierFlags == keymanFlagsForCurrentLayer) {
       keyCode = [self MacKeyCode:nkey.keyCode];
       os_log_debug([KMELogs keyLog], "keyCode: %d, %x for key: %{public}@", keyCode, keyCode, nkey);
       if (keyCode < USHRT_MAX) {
@@ -654,24 +689,16 @@ - (void)setPhysicalShiftState:(BOOL)state {
     _oskShiftState = NO;
 
     os_log_debug([KMELogs oskLog], "hardware shift key released");
-    [self updateKeyLabelsForState];
+    [self updateKeyLabelsForCurrentLayer];
     [self updateShiftKeysForState];
   }
 }
 
 - (void)setOskShiftState:(BOOL)state {
   if (_oskShiftState != state && !self.physicalShiftState) {
     _oskShiftState = state;
-    if (state) {
-      os_log_debug([KMELogs oskLog], "OSK shift key selected");
-      [self updateKeyLabelsForState];
-      [self updateShiftKeysForState];
-    }
-    else if (!self.physicalShiftState) {
-      os_log_debug([KMELogs oskLog], "OSK shift key de-selected");
-      [self updateKeyLabelsForState];
-      [self updateShiftKeysForState];
-    }
+    [self updateKeyLabelsForCurrentLayer];
+    [self updateShiftKeysForState];
   }
 }
 
@@ -684,22 +711,16 @@ - (void)setPhysicalOptionState:(BOOL)state {
      * The state of the physical key overrides the modifier key clicked in the OSK.
      */
     _oskOptionState = NO;
-    [self updateKeyLabelsForState];
+    [self updateKeyLabelsForCurrentLayer];
     [self updateOptionKeysForState];
   }
 }
 
 - (void)setOskOptionState:(BOOL)state {
   if (_oskOptionState != state && !self.physicalOptionState) {
     _oskOptionState = state;
-    if (state) {
-      [self updateKeyLabelsForState];
-      [self updateOptionKeysForState];
-    }
-    else if (!self.physicalOptionState) {
-      [self updateKeyLabelsForState];
-      [self updateOptionKeysForState];
-    }
+    [self updateKeyLabelsForCurrentLayer];
+    [self updateOptionKeysForState];
   }
 }
 
@@ -712,22 +733,16 @@ - (void)setPhysicalControlState:(BOOL)state {
      * The state of the physical key overrides the modifier key clicked in the OSK.
      */
     _oskControlState = NO;
-    [self updateKeyLabelsForState];
+    [self updateKeyLabelsForCurrentLayer];
     [self updateControlKeysForState];
   }
 }
 
 - (void)setOskControlState:(BOOL)state {
   if (_oskControlState != state && !self.physicalControlState) {
     _oskControlState = state;
-    if (state) {
-      [self updateKeyLabelsForState];
-      [self updateControlKeysForState];
-    }
-    else if (!self.physicalControlState) {
-      [self updateKeyLabelsForState];
-      [self updateControlKeysForState];
-    }
+    [self updateKeyLabelsForCurrentLayer];
+    [self updateControlKeysForState];
   }
 }