diff --git a/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m b/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m index dd9a8a3ceea..3dfbd8593b9 100644 --- a/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m +++ b/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m @@ -8,6 +8,7 @@ #import "KMInputMethodEventHandler.h" #import #import /* For kVK_ constants. */ +#import #import "KeySender.h" #import "TextApiCompliance.h" #import "KMSettingsRepository.h" @@ -370,7 +371,7 @@ -(NSString*)readContext:(NSEvent *)event forClient:(id) client { contextString = attributedString.string; //only uncomment for testing as we do not want to write context in logs - //os_log_debug([KMLogs testLog], " length: %lu result: %{public}@", contextString.length, contextString); + //os_log_debug([KMLogs keyTraceLog], " length: %lu result: %{public}@", contextString.length, contextString); } } } @@ -405,18 +406,7 @@ -(BOOL)applyKeyOutputToTextInputClient:(CoreKeyOutput*)output keyDownEvent:(nonn [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; [self insertAndReplaceTextForOutput:output client:client]; } else if (output.isDeleteOnlyScenario) { - if ((event.keyCode == kVK_Delete) && output.codePointsToDeleteBeforeInsert == 1) { - // let the delete pass through in the original event rather than sending a new delete - NSString *message = @"applyOutputToTextInputClient, delete only scenario with passthrough"; - os_log_debug([KMLogs keyTraceLog], "%@", message); - [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; - handledEvent = NO; - } else { - NSString *message = @"applyOutputToTextInputClient, delete only scenario"; - os_log_debug([KMLogs keyTraceLog], "%@", message); - [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; - [self sendEvents:event forOutput:output]; - } + handledEvent = [self handleDeleteOnlyScenario:output keyDownEvent:event client:client]; } else if (output.isDeleteAndInsertScenario) { // TODO: fix issue #10246 /* @@ -511,6 +501,204 @@ -(void)insertAndReplaceText:(NSString *)text deleteCount:(int) replacementCount } } +/** + * Handles deleting without an associated insert in one of three methods: + * 1. delete via replace: do the delete by replacing two (or more) characters with one. + * 2. backspace passthrough : if the original keydown event was a backspace, pass it through unhandled + * 3. generate event: generate keydown backspace events as necessary + */ +-(BOOL)handleDeleteOnlyScenario:(CoreKeyOutput*)output keyDownEvent:(nonnull NSEvent *)event client:(id) client { + + // attempt to delete by replacing -- for compliant apps only + if ([self handleDeleteWithReplacement:output keyDownEvent:event client:client]) { + return YES; + } + + // pass through if this was a backspace keydown event + if (event.keyCode == kVK_Delete && output.codePointsToDeleteBeforeInsert == 1) { + // let the delete pass through in the original event rather than sending a new delete + NSString *message = @"handleDeleteOnlyForOutput, delete only scenario with passthrough"; + os_log_debug([KMLogs keyTraceLog], "%@", message); + [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; + + // instruct system to handle the event + return NO; + } + else { + // otherwise generate a backspace + NSString *message = @"handleDeleteOnlyForOutput, send backspace event"; + os_log_debug([KMLogs keyTraceLog], "%@", message); + [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; + [self sendEvents:event forOutput:output]; + + return YES; + } +} + +/** + * For compliant apps only. + * + * This an attempt to make sure that deletion removes only the expected codepoints. + * When handling a transform which only deletes a character, or when allowing a + * backspace to pass through, the OS or application may not use the same rules + * around deletion as Keyman -- especially when deleting clusters such as letter + + * combining diacritic (e.g. `U+0062 U+0301`), where some applications may delete + * both together as they represent a single 'grapheme cluster'. + * + * (Note, the question of whether it is appropriate for backspace to delete a + * cluster rather than a codepoint from an end-user perspective is not relevant + * here, because what is important is that we match the rules that the keyboard has + * provided, which means we need a method of deleting a precise number of + * codepoints. The keyboard author can and should include rules for cluster + * deletion that meet end-user expectations.) + * + * The `insertText` API takes two parameters: a string to insert, and a range to + * replace with that string. However, we cannot simply pass through a zero-length + * insertion string along with the range to delete, because the `insertText` API + * treats this as an invalid call and ignores it. + * + * Instead, we can delete the desired number of codepoints only by using the + * `insertText` API to replace e.g. two codepoints with one. + * + * This method only works for compliant apps because non-compliant apps do not + * support the `insertText` API. + * + * Ref: https://developer.apple.com/documentation/appkit/nstextinputclient/inserttext(_:replacementrange:) + */ +-(BOOL)handleDeleteWithReplacement:(CoreKeyOutput*)output keyDownEvent:(nonnull NSEvent *)event client:(id) client { + BOOL handledEvent = NO; + NSString *context = [self readContext:event forClient:client]; + + // guard: only for compliant apps with sufficient context + if (!(self.apiCompliance.canReplaceText) || ([context length] <= output.textToDelete.length)) { + os_log_debug([KMLogs keyTraceLog], "cannot replace text, non-compliant or insufficient context"); + return NO; // return without deleting/replacing + } + + // guard: the logic of this method depends on locating textToDelete in the context + if (![self stringToDeleteMatchesContextSuffix:output.textToDelete context:context]) { + os_log_debug([KMLogs keyTraceLog], "cannot replace text, textToDelete not found at end of context"); + return NO; // return without deleting/replacing + } + + NSUInteger deletionTargetLength = output.textToDelete.length; + NSUInteger deletionTargetLocation = context.length-deletionTargetLength; + NSUInteger precedingCharacterLocation = deletionTargetLocation - 1; + + if ([self deletionWillReplacePartOfCluster: deletionTargetLocation precedingCharacterLocation:precedingCharacterLocation context:context]) { + handledEvent = [self deleteByReplacingWithPrecedingCharacter:precedingCharacterLocation deleteLength:deletionTargetLength context:context client:client]; + } else { + handledEvent = [self deleteByReplacingWithPrecedingCluster:precedingCharacterLocation deleteLength:deletionTargetLength context:context client:client]; + } + + return handledEvent; +} + +/** + * Check whether the string to be deleted is found at the tail end of the current context. + */ +-(BOOL) stringToDeleteMatchesContextSuffix:(NSString*)textToDelete context:(NSString*) context { + BOOL doesMatch = NO; + + // get length of string to delete and compare to end of context + NSUInteger deleteLength = textToDelete.length; + NSUInteger locationOfDeletionTarget = context.length-deleteLength; + NSUInteger locationOfPrecedingCharacter = locationOfDeletionTarget - 1; + NSString *contextSuffix = [context substringFromIndex:context.length-deleteLength]; + + os_log_debug([KMLogs keyTraceLog], "stringToDeleteMatchesSuffix, textToDelete: '%{public}@', contextSuffix: '%{public}@', locationOfDeletionTarget: %u, locationOfprecedingCharacter: %u", textToDelete, contextSuffix, (int)locationOfDeletionTarget, (int)locationOfPrecedingCharacter); + + doesMatch = [textToDelete isEqualToString:contextSuffix]; + os_log_debug([KMLogs keyTraceLog], "stringToDeleteMatchesSuffix: %{public}@", doesMatch?@"YES":@"NO"); + return doesMatch; +} + +/** + * Check whether the string to be deleted is part of the same cluster as the character in the context that precedes it. + */ +-(BOOL) deletionWillReplacePartOfCluster: (NSUInteger)deletionLocation precedingCharacterLocation: (NSUInteger)precedingLocation context:(NSString*) context { + // NSString objects hold UTF-16 characters, so a single unicode composed character + // or grapheme cluster may occupy a range of NSString indices instead of a single character. + // This includes base and combining characters potentially composed of surrogate pairs. + NSRange firstDeletionTargetClusterRange = [context rangeOfComposedCharacterSequenceAtIndex: deletionLocation]; + + // get range of the preceding cluster in the context + NSRange precedingClusterRange = [context rangeOfComposedCharacterSequenceAtIndex: precedingLocation]; + + NSString *firstFullCharacterToDelete = [context substringWithRange:firstDeletionTargetClusterRange]; + NSString *precedingFullCharacter = [context substringWithRange:precedingClusterRange]; + os_log_debug([KMLogs keyTraceLog], "firstDeletionTargetCharacterRange: %{public}@, deletionCharacter: %{public}@, precedingCharacterRange %{public}@, precedingCharacter: %{public}@", NSStringFromRange(firstDeletionTargetClusterRange), firstFullCharacterToDelete, NSStringFromRange(precedingClusterRange), precedingFullCharacter); + + // true when the first character to delete and the preceding character + // from the context are part of the same grapheme cluster + return NSEqualRanges(firstDeletionTargetClusterRange, precedingClusterRange); +} + +/** + * Replace both the text to delete and the character preceding it solely with the character that precedes it. + * Returns YES if executing the replace/delete and NO otherwise. + */ +-(BOOL) deleteByReplacingWithPrecedingCharacter:(NSUInteger)precedingCharacterLocation deleteLength:(NSUInteger)deleteLength context:(NSString*) context client:(id) client { + + os_log_debug([KMLogs keyTraceLog], "deleteByReplacingWithPrecedingCharacter, deletion target is part of the same grapheme cluster as the character that precedes it"); + // get the preceding character + NSRange precedingCharacterRange = NSMakeRange(precedingCharacterLocation, 1); + NSString *replacementString = [context substringWithRange:precedingCharacterRange]; + + // guard: if preceding character is a control character, return NO + if ([self containsControlCharacter:replacementString]) { + NSString *message = @"replacementString contains control characters, cannot delete with replace"; + os_log_debug([KMLogs keyTraceLog], "%@", message); + [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; + return NO; + } + + // perform the replacement + NSUInteger replacementLength = [replacementString length] + deleteLength; + NSRange replacementRange = NSMakeRange(precedingCharacterLocation, replacementLength); + os_log_debug([KMLogs keyTraceLog], "replacementRange: %{public}@", NSStringFromRange(replacementRange)); + [client insertText:replacementString replacementRange:replacementRange]; + + return YES; +} + +/** + * Replace both the text to delete and the cluster preceding it solely with the cluster that precedes it. + * The 'cluster' may be just one character, but if contains surrogate pairs, this ensures that they stay together. + * Returns YES if executing the replace/delete and NO otherwise. + */ + -(BOOL) deleteByReplacingWithPrecedingCluster:(NSUInteger)precedingCharacterLocation deleteLength:(NSUInteger)deleteLength context:(NSString*) context client:(id) client { + + os_log_debug([KMLogs keyTraceLog], "deleteByReplacingWithPrecedingCluster, deletion target is independent of the grapheme cluster that precedes it"); + + // get range of the preceding cluster and the substring from the context + NSRange precedingClusterRange = [context rangeOfComposedCharacterSequenceAtIndex: precedingCharacterLocation]; + NSString *replacementString = [context substringWithRange:precedingClusterRange]; + + // guard: if preceding cluster contains control characters, return NO + if ([self containsControlCharacter:replacementString]) { + NSString *message = @"replacementString contains control characters, cannot delete with replace"; + os_log_debug([KMLogs keyTraceLog], "%@", message); + [KMSentryHelper addDebugBreadCrumb:@"event" message:message]; + return NO; + } + + // perform the replacement + NSUInteger replacementLength = [replacementString length] + deleteLength; + NSRange replacementRange = NSMakeRange([context length] - replacementLength, replacementLength); + os_log_debug([KMLogs keyTraceLog], "replacementRange: %{public}@", NSStringFromRange(replacementRange)); + [client insertText:replacementString replacementRange:replacementRange]; + + return YES; +} + +-(BOOL) containsControlCharacter:(NSString*)text { + NSCharacterSet *controlSet = [NSCharacterSet controlCharacterSet]; + NSRange range = [text rangeOfCharacterFromSet:controlSet]; + + return (range.location != NSNotFound); +} + /** * Calculates the range where text will be inserted and replace existing text. * Returning {NSNotFound, NSNotFound} for range signifies to insert at current location without replacement. diff --git a/mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.h b/mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.h index 1f5b55f17fa..40c516a8dbc 100644 --- a/mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.h +++ b/mac/Keyman4MacIM/Keyman4MacIM/TextApiCompliance.h @@ -22,6 +22,7 @@ NS_ASSUME_NONNULL_BEGIN -(void) checkComplianceAfterInsert:(NSString *)insertedText deleted:(NSString *)deletedText; -(BOOL)isComplianceUncertain; -(BOOL)canReadText; +-(BOOL)canReplaceText; -(BOOL)mustBackspaceUsingEvents; @end