feat(mac): renaming and moved adjustModifiers to KMModifierMapping

Fixes: #875

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.h

@@ -62,23 +62,25 @@ static const int KEYMAN_FIRST_KEYBOARD_MENUITEM_INDEX = 0;
 }
 
 @property (nonatomic, strong) KMEngine *kme;
-@property (nonatomic, strong) KMXFile *kmx;
-@property (nonatomic, strong) KMModifierMapping *modifierMapping;
+// TODO: refactor and encapsulate below properties with current keyboard
+@property (nonatomic, strong, readonly) KMXFile *kmx;
+@property (nonatomic, strong, readonly) KMModifierMapping *modifierMapping;
 @property (nonatomic, strong) KVKFile *kvk;
+@property (nonatomic, strong) NSString *keyboardName;
+@property (nonatomic, strong) NSString *selectedKeyboard;
+@property (nonatomic, strong) NSImage *keyboardIcon;
+// TODO: refactor above properties
 @property (nonatomic, strong) NSString *keyboardsPath;
 @property (nonatomic, strong) NSString *fontsPath;
 @property (nonatomic, strong) NSMutableArray *kmxFileList;
-@property (nonatomic, strong) NSString *selectedKeyboard;
 @property (nonatomic, strong) NSMutableArray *activeKeyboards;
 @property (assign) int numberOfKeyboardMenuItems;
 @property (nonatomic, strong) NSMutableString *contextBuffer;
-@property (nonatomic, assign) NSEventModifierFlags currentModifierFlags;
+@property (nonatomic, assign) NSEventModifierFlags currentModifiers;
 @property (nonatomic, assign) CFMachPortRef lowLevelEventTap;
 @property (nonatomic, assign) CFRunLoopSourceRef runLoopEventSrc;
 @property (nonatomic, assign) BOOL contextChangedByLowLevelEvent;
 @property (nonatomic, strong) OSKWindowController *oskWindow;
-@property (nonatomic, strong) NSString *keyboardName;
-@property (nonatomic, strong) NSImage *keyboardIcon;
 @property (nonatomic, strong) NSAlert *downloadInfoView;
 @property (nonatomic, strong) NSProgressIndicator *progressIndicator;
 @property (nonatomic, weak) KMInputController *inputController;
@@ -91,7 +93,6 @@ static const int KEYMAN_FIRST_KEYBOARD_MENUITEM_INDEX = 0;
 @property (nonatomic, strong) NSString *downloadFilename;
 @property (nonatomic, strong) NSMutableData *receivedData;
 @property (nonatomic, assign) NSUInteger expectedBytes;
-@property (nonatomic, assign) BOOL useNullChar;
 
 - (NSMenu *)menu;
 - (void)saveActiveKeyboards;
@@ -101,6 +102,9 @@ static const int KEYMAN_FIRST_KEYBOARD_MENUITEM_INDEX = 0;
 - (void)showConfigurationWindow;
 - (void)selectKeyboardFromMenu:(NSInteger)tag;
 - (void)handleKeyEvent:(NSEvent *)event;
+- (void)loadKeyboardFromKmxFile:(KMXFile *)kmx;
+- (void)resetKmx;
+- (NSEventModifierFlags) determineModifiers;
 - (BOOL)unzipFile:(NSString *)filePath;
 - (NSWindowController *)downloadKBWindow_;
 - (NSWindowController *)aboutWindow_;

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.m

@@ -284,8 +284,8 @@ CGEventRef eventTapFunction(CGEventTapProxy proxy, CGEventType type, CGEventRef
     switch (type) {
       case kCGEventFlagsChanged:
         os_log_debug([KMLogs eventsLog], "eventTapFunction: system event kCGEventFlagsChanged to: 0x%X", (int) sysEvent.modifierFlags);
-        appDelegate.currentModifierFlags = sysEvent.modifierFlags;
-        if (appDelegate.currentModifierFlags & NSEventModifierFlagCommand) {
+        appDelegate.currentModifiers = sysEvent.modifierFlags;
+        if (appDelegate.currentModifiers & NSEventModifierFlagCommand) {
           appDelegate.contextChangedByLowLevelEvent = YES;
         }
         break;
@@ -389,7 +389,12 @@ - (KMPackageReader *)packageReader {
   return _packageReader;
 }
 
-- (void)setKmx:(KMXFile *)kmx {
+- (void)resetKmx {
+  _kmx = nil;
+  _modifierMapping = nil;
+}
+
+- (void)loadKeyboardFromKmxFile:(KMXFile *)kmx {
   _kmx = kmx;
   CoreKeyboardInfo *keyboardInfo = [self.kme loadKeyboardFromKmxFile:kmx];
 
@@ -790,7 +795,7 @@ - (void) setSelectedKeyboard:(NSString*)keyboardName inMenuItem:(NSMenuItem*) me
   os_log_debug([KMLogs dataLog], "setSelectedKeyboard, keyboardName = '%{public}@', full path = '%{public}@'", keyboardName, fullPath);
   [menuItem setState:NSOnState];
   KMXFile *kmx = [[KMXFile alloc] initWithFilePath:fullPath];
-  [self setKmx:kmx];
+  [self loadKeyboardFromKmxFile:kmx];
   NSDictionary *kmxInfo = [KMXFile keyboardInfoFromKmxFile:fullPath];
   NSString *kvkFilename = [kmxInfo objectForKey:kKMVisualKeyboardKey];
   if (kvkFilename != nil) {
@@ -818,7 +823,7 @@ - (void) addKeyboardPlaceholderMenuItem {
   NSString* placeholder = NSLocalizedString(@"no-keyboard-configured-menu-placeholder", nil);
   NSMenuItem *item = [[NSMenuItem alloc] initWithTitle:placeholder action:NULL keyEquivalent:@""];
   [self.menu insertItem:item atIndex:KEYMAN_FIRST_KEYBOARD_MENUITEM_INDEX];
-  [self setKmx:nil];
+  [self resetKmx];
   [self setKvk:nil];
   [self setKeyboardName:nil];
   [self setKeyboardIcon:nil];
@@ -847,7 +852,7 @@ - (void)selectKeyboardFromMenu:(NSInteger)tag {
   os_log_debug([KMLogs dataLog], "setSelectedKeyboard, keyboardName = '%{public}@', full path = '%{public}@'", path, fullPath);
 
   KMXFile *kmx = [[KMXFile alloc] initWithFilePath:fullPath];
-  [self setKmx:kmx];
+  [self loadKeyboardFromKmxFile:kmx];
   KVKFile *kvk = nil;
   NSDictionary *kmxInfo = [KMXFile keyboardInfoFromKmxFile:fullPath];
   NSString *kvkFilename = [kmxInfo objectForKey:kKMVisualKeyboardKey];
@@ -1225,6 +1230,11 @@ - (void)handleKeyEvent:(NSEvent *)event {
   [_oskWindow.oskView handleKeyEvent:event];
 }
 
+- (NSEventModifierFlags) determineModifiers {
+  NSEventModifierFlags originalModifiers = self.currentModifiers;
+  return [self.modifierMapping adjustModifiers:originalModifiers];
+}
+
 extern const CGKeyCode kProcessPendingBuffer;
 
 // This could more easily and logically be done in the input method, but this

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m

@@ -119,50 +119,22 @@ - (BOOL) handleEventWithKeymanEngine:(NSEvent *)event in:(id) sender {
   return [self applyKeymanCoreActions:output event:event client:sender];
 }
 
-/**
- * If necessary change the modifier so that it appears that the right option key was pressed instead of the
- * left option key. This will trigger keyboard rules that are defined for right option because both option keys
- * generally have the same meaning on the Mac.
- */
-- (NSEventModifierFlags) determineModifierFlag {
-  const NSEventModifierFlags LEFT_OPTION_MASK = 0x80120;
-  const NSEventModifierFlags RIGHT_OPTION_MASK = 0x80140;
-
-  NSEventModifierFlags originalModifierFlag = self.appDelegate.currentModifierFlags;
-  NSEventModifierFlags newModifierFlag = originalModifierFlag;
-
-  if (self.appDelegate.modifierMapping.bothOptionKeysGenerateRightAlt) {
-    // if original includes left option key, then replace it with the right option key
-    if ((originalModifierFlag & LEFT_OPTION_MASK) == LEFT_OPTION_MASK) {
-      os_log_debug([KMLogs eventsLog], "determineModifierFlag \n  originalModifierFlag: 0x%lX \n  LEFT_OPTION_MASK: 0x%lx \n  inverse: 0x%lx", (unsigned long)originalModifierFlag, LEFT_OPTION_MASK, ~LEFT_OPTION_MASK);
-      // clear all left option bits
-      newModifierFlag = originalModifierFlag & ~LEFT_OPTION_MASK;
-      os_log_debug([KMLogs eventsLog], "  cleared left option bits: 0x%lX", (unsigned long)newModifierFlag);
-      // set all right option bits
-      newModifierFlag = newModifierFlag | RIGHT_OPTION_MASK;
-      os_log_debug([KMLogs eventsLog], "  set right option bits: 0x%lX", (unsigned long)newModifierFlag);
-    }
-  } else {
-    os_log_debug([KMLogs eventsLog], "determineModifierFlag = originalModifierFlag: 0x%lX", (unsigned long)originalModifierFlag);
-  }
-  return newModifierFlag;
-}
-
 - (CoreKeyOutput*) processEventWithKeymanEngine:(NSEvent *)event in:(id) sender {
   CoreKeyOutput* coreKeyOutput = nil;
   if (self.appDelegate.lowLevelEventTap != nil) {
-    NSEventModifierFlags newModifierFlag = [self determineModifierFlag];
-    NSEvent *eventWithOriginalModifierFlags = [NSEvent keyEventWithType:event.type location:event.locationInWindow modifierFlags:newModifierFlag timestamp:event.timestamp windowNumber:event.windowNumber context:[NSGraphicsContext currentContext] characters:event.characters charactersIgnoringModifiers:event.charactersIgnoringModifiers isARepeat:event.isARepeat keyCode:event.keyCode];
+    NSEventModifierFlags newModifiers = [self.appDelegate determineModifiers];
+    NSEvent *eventWithOriginalModifierFlags = [NSEvent keyEventWithType:event.type location:event.locationInWindow modifierFlags:newModifiers timestamp:event.timestamp windowNumber:event.windowNumber context:[NSGraphicsContext currentContext] characters:event.characters charactersIgnoringModifiers:event.charactersIgnoringModifiers isARepeat:event.isARepeat keyCode:event.keyCode];
     coreKeyOutput = [self.kme processEvent:eventWithOriginalModifierFlags];
-    os_log_debug([KMLogs eventsLog], "processEventWithKeymanEngine, using newModifierFlag 0x%lX, instead of event.modifiers 0x%lX", newModifierFlag, event.modifierFlags);
+    os_log_debug([KMLogs eventsLog], "processEventWithKeymanEngine, using newModifierFlag 0x%lX, instead of event.modifiers 0x%lX", newModifiers, event.modifierFlags);
   }
   else {
-    // Depending on the client app and the keyboard, using the passed-in event as it is should work okay.
-    // Keyboards that depend on chirality support will not work. And command-key actions that change the
-    // context might go undetected in some apps, resulting in errant behavior for subsequent typing.
+    /** 
+     * Without the low level event tap, there will be no way to distinguish left and right option keys.
+     * Also command-key actions that should invalidate the context may go undetected.
+     * Have yet to determine which scenarios prevent an event tap from be created
+     */
     coreKeyOutput = [self.kme processEvent:event];
   }
-  //os_log_debug([KMLogs eventsLog], "processEventWithKeymanEngine, coreKeyOutput = %{public}@", coreKeyOutput);
   return coreKeyOutput;
 }
 

mac/Keyman4MacIM/Keyman4MacIM/KMModifierMapping.h

@@ -14,6 +14,7 @@ NS_ASSUME_NONNULL_BEGIN
 -(instancetype)init:(CoreKeyboardInfo*)keyboardInfo;
 -(BOOL)optionKeysUnused;
 -(BOOL)bothOptionKeysGenerateRightAlt;
+-(NSEventModifierFlags)adjustModifiers:(NSEventModifierFlags)originalModifiers;
 @end
 
 NS_ASSUME_NONNULL_END

mac/Keyman4MacIM/Keyman4MacIM/KMModifierMapping.m

@@ -10,13 +10,21 @@
 
 #import "KMModifierMapping.h"
 #import <KeymanEngine4Mac/KeymanEngine4Mac.h>
+#import "KMLogs.h"
 
 @interface KMModifierMapping ()
 @property (nonatomic, strong) CoreKeyboardInfo *keyboardInfo;
 @end
 
 @implementation KMModifierMapping
 
+const NSEventModifierFlags LEFT_OPTION_MASK = 0x80120;
+const NSEventModifierFlags RIGHT_OPTION_MASK = 0x80140;
+
++(BOOL)containsLeftOptionFlag:(NSEventModifierFlags)modifiers {
+  return (modifiers & LEFT_OPTION_MASK) == LEFT_OPTION_MASK;
+}
+
 -(instancetype)init:(CoreKeyboardInfo*)keyboardInfo {
   self = [super init];
   if (self) {
@@ -35,4 +43,30 @@ -(BOOL)bothOptionKeysGenerateRightAlt {
   return (self.keyboardInfo.containsRightAlt || self.keyboardInfo.containsAlt) && !self.keyboardInfo.containsLeftAlt;
 }
 
+/**
+ * If necessary, change the modifiers so that it appears that the right option key was pressed instead of the
+ * left option key. This will trigger keyboard rules that are defined for right option because both option keys
+ * usually have the same meaning on the Mac.
+ */
+- (NSEventModifierFlags) adjustModifiers:(NSEventModifierFlags)originalModifiers {
+
+  NSEventModifierFlags newModifiers = originalModifiers;
+
+  if (self.bothOptionKeysGenerateRightAlt) {
+    // if original includes left option key, then replace it with the right option key
+    if ([KMModifierMapping containsLeftOptionFlag:originalModifiers]) {
+
+      // clear all left option bits
+      newModifiers = originalModifiers & ~LEFT_OPTION_MASK;
+
+      // set all right option bits
+      newModifiers = newModifiers | RIGHT_OPTION_MASK;
+      os_log_debug([KMLogs eventsLog], "adjustModifiers, changing from originalModifiers: 0x%lX to newModifiers: 0x%lX", originalModifiers, newModifiers);
+    }
+  } else {
+    os_log_debug([KMLogs eventsLog], "adjustModifiers, retaining originalModifiers: 0x%lX", (unsigned long)originalModifiers);
+  }
+  return newModifiers;
+}
+
 @end

mac/Keyman4MacIM/KeymanTests/TestAppDelegate.h

@@ -21,7 +21,6 @@ typedef void(^PostEventCallback)(CGEventRef eventToPost);
 @property (nonatomic, strong) NSMutableString *contextBuffer;
 @property (nonatomic, assign) CFMachPortRef lowLevelEventTap; // Always nil for tests
 @property (nonatomic, assign) BOOL contextChangingEventDetected;
-@property (nonatomic, assign) BOOL useNullChar;
 @property (nonatomic, assign) CGKeyCode virtualKeyPosted;
 
 // Helper method

mac/Keyman4MacIM/KeymanTests/TestAppDelegate.m

@@ -24,7 +24,7 @@ - (KMEngine *)kme {
 
 - (void)setKmx:(KMXFile *)kmx {
   _kmx = kmx;
-  [self.kme setKmx:_kmx];
+  [self.kme loadKeyboardFromKmxFile:_kmx];
 }
 
 - (NSMutableString *)contextBuffer {