improve compliance detection, accounting for all delete and insert co…

…mbinations

mac/Keyman4MacIM/Keyman4MacIM.xcodeproj/project.pbxproj

@@ -27,6 +27,7 @@
 		299B5E2129126CA20070841D /* keyman-icon.png in Resources */ = {isa = PBXBuildFile; fileRef = 299B5E1F29126CA20070841D /* keyman-icon.png */; };
 		29B42A572728339900EDD5D3 /* KMInfoWindowController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 29B42A592728339900EDD5D3 /* KMInfoWindowController.xib */; };
 		29B42A602728343B00EDD5D3 /* KMKeyboardHelpWindowController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 29B42A622728343B00EDD5D3 /* KMKeyboardHelpWindowController.xib */; };
+		29B6FB732BC39DD60074BF7F /* TextApiComplianceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 29B6FB722BC39DD60074BF7F /* TextApiComplianceTests.m */; };
 		37A245C12565DFA6000BBF92 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 37A245C02565DFA6000BBF92 /* Assets.xcassets */; };
 		37AE5C9D239A7B770086CC7C /* qrcode.min.js in Resources */ = {isa = PBXBuildFile; fileRef = 37AE5C9C239A7B770086CC7C /* qrcode.min.js */; };
 		37C2B0CB25FF2C350092E16A /* Help in Resources */ = {isa = PBXBuildFile; fileRef = 37C2B0CA25FF2C340092E16A /* Help */; };
@@ -227,6 +228,7 @@
 		29B6BB5E292239B4008F04BD /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/KMKeyboardHelpWindowController.strings"; sourceTree = "<group>"; };
 		29B6BB5F292239B4008F04BD /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/MainMenu.strings"; sourceTree = "<group>"; };
 		29B6BB60292239DC008F04BD /* zh-Hans */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "zh-Hans"; path = "zh-Hans.lproj/Localizable.strings"; sourceTree = "<group>"; };
+		29B6FB722BC39DD60074BF7F /* TextApiComplianceTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = TextApiComplianceTests.m; sourceTree = "<group>"; };
 		29B70A23283CB6680086B580 /* es-419 */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "es-419"; path = "es-419.lproj/KMAboutWindowController.strings"; sourceTree = "<group>"; };
 		29B70A24283CB6690086B580 /* es-419 */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "es-419"; path = "es-419.lproj/preferences.strings"; sourceTree = "<group>"; };
 		29B70A25283CB66A0086B580 /* es-419 */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "es-419"; path = "es-419.lproj/KMInfoWindowController.strings"; sourceTree = "<group>"; };
@@ -682,6 +684,7 @@
 				E211769E20E18C0B00F8065D /* AppleCompliantTestClient.h */,
 				E211769F20E18C5200F8065D /* AppleCompliantTestClient.m */,
 				2999C69829D3E4CE006366F5 /* InputMethodTests.m */,
+				29B6FB722BC39DD60074BF7F /* TextApiComplianceTests.m */,
 			);
 			path = KeymanTests;
 			sourceTree = "<group>";
@@ -1002,6 +1005,7 @@
 				2999C69929D3E4CE006366F5 /* InputMethodTests.m in Sources */,
 				E211769D20E182DD00F8065D /* NoContextTestClient.m in Sources */,
 				E2D5529D20DD8E0200D77CFB /* LegacyTestClient.m in Sources */,
+				29B6FB732BC39DD60074BF7F /* TextApiComplianceTests.m in Sources */,
 				E21176A020E18C5200F8065D /* AppleCompliantTestClient.m in Sources */,
 				E2585A8120DD7CF100CBB994 /* TestAppDelegate.m in Sources */,
 				E28914D62024E4010048E71E /* KMInputMethodEventHandler.m in Sources */,

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m

@@ -199,7 +199,7 @@ - (void)checkTextApiCompliance:(id)client {
     [self.appDelegate logDebugMessage:@"KMInputMethodHandler initWithClient checkTextApiCompliance: %@", _apiCompliance];
   } else if (self.apiCompliance.isComplianceUncertain) {
     // We have a valid TextApiCompliance object, but compliance is uncertain, so test it
-    [self.apiCompliance testCompliance:client];
+    [self.apiCompliance checkCompliance:client];
   }
 }
 
@@ -454,7 +454,7 @@ -(void)insertAndReplaceText:(NSString *)text deleteCount:(int) replacementCount
 
   [client insertText:text replacementRange:replacementRange];
   if (self.apiCompliance.isComplianceUncertain) {
-    [self.apiCompliance testComplianceAfterInsert:client];
+    [self.apiCompliance checkComplianceAfterInsert:client delete:textToDelete insert:text];
   }
 }
 

mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.h

@@ -16,8 +16,8 @@ NS_ASSUME_NONNULL_BEGIN
 @property (readonly) NSString *clientApplicationId;
 
 -(instancetype)initWithClient:(id) client applicationId:(NSString *)appId;
--(void)testCompliance:(id) client;
--(void)testComplianceAfterInsert:(id) client;
+-(void)checkCompliance:(id) client;
+-(void) checkComplianceAfterInsert:(id) client delete:(NSString *)textToDelete insert:(NSString *)textToInsert;
 -(BOOL)isComplianceUncertain;
 -(BOOL)canReadText;
 -(BOOL)mustBackspaceUsingEvents;

mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.m

@@ -64,7 +64,7 @@ -(instancetype)initWithClient:(id) client applicationId:(NSString *)appId {
 
     // if we do not have hard-coded noncompliance, then test the app
     if (![self applyNoncompliantAppLists:appId]) {
-      [self testCompliance:client];
+      [self checkCompliance:client];
     }
   }
   return self;
@@ -76,77 +76,116 @@ -(NSString *)description
 }
 
 /** test to see if the API selectedRange functions properly for the text input client  */
--(void) testCompliance:(id) client {
-  BOOL selectionApiVerified = NO;
-
+-(void) checkCompliance:(id) client {
   // confirm that the API actually exists (this always seems to return true)
-  selectionApiVerified = [client respondsToSelector:@selector(selectedRange)];
+  self.hasCompliantSelectionApi = [client respondsToSelector:@selector(selectedRange)];
 
-  if (!selectionApiVerified) {
+  if (!self.hasCompliantSelectionApi) {
     self.complianceUncertain = NO;
-    self.hasCompliantSelectionApi = NO;
   }
   else {
     // if API exists, call it and see if it works as expected
-    NSRange selectionRange = [client selectedRange];
-    self.initialSelection = selectionRange;
-    [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance, location = %lu, length = %lu", selectionRange.location, selectionRange.length];
-
-    if (selectionRange.location == NSNotFound) {
-      // NSNotFound may just mean that we don't have the focus yet; say NO for
-      // now, but this may toggle back to YES after the first insertText
-      selectionApiVerified = NO;
-      self.complianceUncertain = YES;
-      [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance not compliant but uncertain, range is NSNotFound"];
-    } else if (selectionRange.location >= 0) {
-      // location greater than or equal to zero may just mean that the client
-      // returns an inaccurate value; say YES for now, but set back to NO if the
-      // first insertText does not cause a change in the selection
-      selectionApiVerified = YES;
-      self.complianceUncertain = YES;
-      [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance compliant but uncertain, location >= 0"];
-    }
+    self.initialSelection = [client selectedRange];
+    [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance, location = %lu, length = %lu", self.initialSelection.location, self.initialSelection.length];
+    [self checkComplianceUsingInitialSelection];
   }
-  [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance workingSelectionApi for app %@: set to %@", self.clientApplicationId, selectionApiVerified?@"yes":@"no"];
+  [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance workingSelectionApi for app %@: set to %@", self.clientApplicationId, self.complianceUncertain?@"YES":@"NO"];
+}
 
-  self.hasCompliantSelectionApi = selectionApiVerified;
+-(void) checkComplianceUsingInitialSelection {
+  if (self.initialSelection.location == NSNotFound) {
+    /**
+     * NSNotFound may just mean that we don't have the focus yet; say NO for now
+     * now, but this may toggle back to YES after the first insertText
+     */
+    self.hasCompliantSelectionApi = NO;
+    self.complianceUncertain = YES;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance not compliant but uncertain, range is NSNotFound"];
+  } else if (self.initialSelection.location >= 0) {
+    /**
+     * location greater than or equal to zero may just mean that the client
+     * returns an inaccurate value; say YES for now, but set back to NO if the
+     * if the selection is not consistent with the first insert
+     */
+    self.hasCompliantSelectionApi = YES;
+    self.complianceUncertain = YES;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance testCompliance compliant but uncertain, location >= 0"];
+  }
 }
 
-/** if complianceUncertain is true, checking the selection after an insert can make it clear  */
--(void) testComplianceAfterInsert:(id) client {
-  if(self.complianceUncertain) {
-    NSRange selectionRange = [client selectedRange];
-    [self.appDelegate logDebugMessage:@"TextApiCompliance testSelectionApiAfterInsert, location = %lu, length = %lu", selectionRange.location, selectionRange.length];
-
-    if (selectionRange.location == NSNotFound) {
-      // NO for certain, insertText means we have focus, NSNotFound means that the selection API does not work
-      self.hasCompliantSelectionApi = NO;
-      self.complianceUncertain = NO;
-      [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert certain, non-compliant, range is NSNotFound"];
-    } else if (selectionRange.location == 0) {
-      // NO for certain, after an insertText we cannot be at location 0
-      self.hasCompliantSelectionApi = NO;
-      self.complianceUncertain = NO;
-      [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert certain, non-compliant, location = 0"];
-    } else if (selectionRange.location > 0) {
-      if (NSEqualRanges(selectionRange, self.initialSelection)) {
-        // NO for certain, when the selection is unchanged after an insert
-        self.hasCompliantSelectionApi = NO;
-        self.complianceUncertain = NO;
-        [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert certain, non-compliant, selection the same: initial location = %@, new location = %@", NSStringFromRange(self.initialSelection), NSStringFromRange(selectionRange)];
-      } else {
-        // YES for certain, when the selection is changed after an insert
-        self.hasCompliantSelectionApi = YES;
-        self.complianceUncertain = NO;
-        [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert certain, compliant, selection changed from initial location = %@ to new location = %@", NSStringFromRange(self.initialSelection), NSStringFromRange(selectionRange)];
-      }
-    }
-
-    [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert, self.hasWorkingSelectionApi = %@ for app %@", self.hasCompliantSelectionApi?@"yes":@"no", self.clientApplicationId];
-    [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert TextApiCompliance: %@", self];
- } else {
-   [self.appDelegate logDebugMessage:@"TextApiCompliance testSelectionApiAfterInsert, compliance is already known"];
+/**
+ * If complianceUncertain is true, checking the location after an insert can confirm whether the app is compliant.
+ * Delete and insert are what core instructed us to do.
+ * Text that was selected in the client when the key was processed is irrelevant as it does not affect the location.
+ */
+-(void) checkComplianceAfterInsert:(id) client delete:(NSString *)textToDelete insert:(NSString *)textToInsert {
+
+  // return if compliance is already certain
+  if(!self.complianceUncertain) {
+    [self.appDelegate logDebugMessage:@"TextApiCompliance testSelectionApiAfterInsert, compliance is already known"];
+    return;
+  }
+
+  NSRange selectionRange = [client selectedRange];
+  if ([self validateNewLocation:selectionRange.location delete:textToDelete]) {
+    BOOL changeExpected = [self isLocationChangeExpectedOnInsert:textToDelete insert:textToInsert];
+    BOOL locationChanged = [self hasLocationChanged:selectionRange];
+    [self validateLocationChange:changeExpected hasLocationChanged:locationChanged];
   }
+
+  [self.appDelegate logDebugMessage:@"TextApiCompliance testComplianceAfterInsert, self.hasWorkingSelectionApi = %@ for app %@", self.hasCompliantSelectionApi?@"YES":@"NO", self.clientApplicationId];
+}
+
+- (BOOL)validateNewLocation:(NSUInteger)location delete:(NSString *)textToDelete  {
+  BOOL validLocation = NO;
+
+  if (location == NSNotFound) {
+    // invalid location: insertText means we have focus, NSNotFound means selection API not functioning
+    self.hasCompliantSelectionApi = NO;
+    self.complianceUncertain = NO;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance validateNewLocation = NO, location is NSNotFound"];
+  } else if ((location == 0) && (textToDelete.length > 0)) {
+    // invalid location: cannot have text to delete at location zero
+    self.hasCompliantSelectionApi = NO;
+    self.complianceUncertain = NO;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance validateNewLocation = NO, location is zero with textToDelete.length > 0"];
+  } else {
+    // location is valid, but do not know if it is compliant yet
+    validLocation = YES;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance validateNewLocation = YES, newLocation = %d, oldLocation = %d", location, self.initialSelection.location];
+  }
+  return validLocation;
+}
+
+- (void) validateLocationChange:(BOOL) changeExpected hasLocationChanged:(BOOL) locationChanged {
+
+  if (changeExpected == locationChanged) {
+    // YES for certain, the location is where we expect it
+    self.hasCompliantSelectionApi = YES;
+    self.complianceUncertain = NO;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance validateLocationChange compliant, locationChanged = %@, changeExpected = %@", locationChanged?@"YES":@"NO", changeExpected?@"YES":@"NO"];
+  } else if (changeExpected != locationChanged) {
+    // NO for certain, when the selection is unchanged after an insert
+    self.hasCompliantSelectionApi = NO;
+    self.complianceUncertain = NO;
+    [self.appDelegate logDebugMessage:@"TextApiCompliance validateLocationChange non-compliant, locationChanged = %@, changeExpected = %@", locationChanged?@"YES":@"NO", changeExpected?@"YES":@"NO"];
+  }
+}
+
+- (BOOL)isLocationChangeExpectedOnInsert:(NSString *)textToDelete insert:(NSString *)textToInsert {
+  BOOL changeExpected = textToInsert.length != textToDelete.length;
+  [self.appDelegate logDebugMessage:@"TextApiCompliance isLocationChangeExpected, changeExpected = %@", changeExpected?@"YES":@"NO"];
+
+  return changeExpected;
+}
+
+- (BOOL)hasLocationChanged:(NSRange)newSelection {
+  NSUInteger newLocation = newSelection.location;
+  NSUInteger oldLocation = self.initialSelection.location;
+  BOOL locationChanged = newLocation != oldLocation;
+  [self.appDelegate logDebugMessage:@"TextApiCompliance hasLocationChanged, currentSelection = %lu, length = %lu", newSelection.location, newSelection.length];
+
+  return locationChanged;
 }
 
 /**

mac/Keyman4MacIM/KeymanTests/InputMethodTests.m

@@ -13,20 +13,12 @@
 #import "KMInputMethodEventHandler.h"
 #import "AppleCompliantTestClient.h"
 #import "TextApiCompliance.h"
-#import "AppleCompliantTestClient.h"
 
 KMInputMethodEventHandler *testEventHandler = nil;
 
 @interface InputMethodTests : XCTestCase
 @end
 
-// included following interface that we can see and test private methods of TextApiCompliance
-@interface TextApiCompliance (Testing)
-
-- (BOOL)arrayContainsApplicationId:(NSString *)clientAppId fromArray:(NSArray *)legacyApps;
-
-@end
-
 @interface KMInputMethodEventHandler (Testing)
 
 - (instancetype)initWithClient:(NSString *)clientAppId client:(id) sender;
@@ -46,29 +38,6 @@ - (void)tearDown {
     // Put teardown code here. This method is called after the invocation of each test method in the class.
 }
 
-- (void)testIsClientAppLegacy_unlistedClientAppId_returnsNo {
-  id client = [[AppleCompliantTestClient alloc] init];
-  NSString *clientAppId = @"com.compliant.app";
-  TextApiCompliance *apiCompliance = [[TextApiCompliance alloc]initWithClient:client applicationId:clientAppId];
-
-  NSArray *legacyAppsArray = [NSArray arrayWithObjects:@"com.microsoft.VSCode",@"com.adobe.Photoshop",nil];
-
-  BOOL isLegacy = [apiCompliance arrayContainsApplicationId:clientAppId fromArray:legacyAppsArray];
-  NSLog(@"isLegacy = %@", isLegacy?@"yes":@"no");
-    XCTAssertFalse(isLegacy, @"App not expected to be in legacy list");
-}
-
-- (void)testIsClientAppLegacy_listedClientAppId_returnsYes {
-  id client = [[AppleCompliantTestClient alloc] init];
-  NSString *clientAppId = @"com.microsoft.VSCode";
-  TextApiCompliance *apiCompliance = [[TextApiCompliance alloc]initWithClient:client applicationId:clientAppId];
-
-  NSArray *legacyAppsArray = [NSArray arrayWithObjects:@"com.adobe.Photoshop",@"com.microsoft.VSCode",nil];
-
-  BOOL isLegacy = [apiCompliance arrayContainsApplicationId:clientAppId fromArray:legacyAppsArray];
-  XCTAssertTrue(isLegacy, @"App expected to be in legacy list");
-}
-
 - (void)testCalculateInsertRange_noDelete_returnsCurrentLocation {
   NSRange selectionRange = NSMakeRange(1, 0);
   NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"" selectionRange:selectionRange];