From 9d7549cd910b7d14e9c5fa42f95478ee61ca8737 Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 15 Feb 2024 10:27:42 +0700 Subject: [PATCH 1/4] fix(ios): context synchronization after new paragraph --- .../KMEI/KeymanEngine/Classes/TextField.swift | 22 ---------- .../KMEI/KeymanEngine/Classes/TextView.swift | 16 ------- web/src/app/webview/src/configuration.ts | 7 --- web/src/app/webview/src/contextManager.ts | 44 ++++++++++++++----- web/src/app/webview/src/keymanEngine.ts | 9 +++- 5 files changed, 39 insertions(+), 59 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift b/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift index e1cf9f116c3..2073e221499 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/TextField.swift @@ -19,7 +19,6 @@ public class TextField: UITextField, KeymanResponder { public var isInputClickSoundEnabled = true private var delegateProxy: TextFieldDelegateProxy! - private var shouldUpdateKMText = false private var keyboardChangedObserver: NotificationObserver? @@ -52,8 +51,6 @@ public class TextField: UITextField, KeymanResponder { self.inputView = Manager.shared.inputViewController.view - NotificationCenter.default.addObserver(self, selector: #selector(self.textFieldTextDidChange), - name: UITextField.textDidChangeNotification, object: self) keyboardChangedObserver = NotificationCenter.default.addObserver(forName: Notifications.keyboardChanged, observer: self, function: TextField.keyboardChanged) @@ -155,19 +152,6 @@ public class TextField: UITextField, KeymanResponder { @objc func enableInputClickSound() { isInputClickSoundEnabled = true } - - @objc func textFieldTextDidChange(_ notification: Notification) { - if shouldUpdateKMText { - // Catches copy/paste operations - let textRange = selectedTextRange! - let newRange = NSRange(location: offset(from: beginningOfDocument, to: textRange.start), - length: offset(from: textRange.start, to: textRange.end)) - - Manager.shared.setContextState(text: text, range: newRange) - // This is called when editing in-app; do not reset context here. - shouldUpdateKMText = false - } - } } extension KeymanResponder where Self: TextField { @@ -196,12 +180,6 @@ extension KeymanResponder where Self: TextField { // MARK: - UITextFieldDelegate extension TextField: UITextFieldDelegate { - public func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, - replacementString string: String) -> Bool { - shouldUpdateKMText = true // Enable text update to catch copy/paste operations - return true - } - public func textFieldShouldClear(_ textField: UITextField) -> Bool { if textField == self { Manager.shared.clearText() diff --git a/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift b/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift index 46673a4e17f..7d67d720373 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/TextView.swift @@ -262,22 +262,6 @@ extension TextView: UITextViewDelegate { os_log("%{public}s", log:KeymanEngineLogger.ui, type: .debug, messageTwo) } - public func textView(_ textView: UITextView, shouldChangeTextIn range: NSRange, - replacementText text: String) -> Bool { - // Enable text update to catch copy/paste operations - shouldUpdateKMText = true - return true - } - - public func textViewDidChange(_ textView: UITextView) { - if shouldUpdateKMText { - // Catches copy/paste operations - Manager.shared.setContextState(text: textView.text, range: textView.selectedRange) - // This is called when editing in-app; do not reset context here. - shouldUpdateKMText = false - } - } - public func textViewShouldEndEditing(_ textView: UITextView) -> Bool { if textView == self { resignFirstResponder() diff --git a/web/src/app/webview/src/configuration.ts b/web/src/app/webview/src/configuration.ts index bcb59c3e1a6..04d847b3ff1 100644 --- a/web/src/app/webview/src/configuration.ts +++ b/web/src/app/webview/src/configuration.ts @@ -58,13 +58,6 @@ export class WebviewConfiguration extends EngineConfiguration { return baseReport; } - - onRuleFinalization(ruleBehavior: RuleBehavior) { - if(!isEmptyTransform(ruleBehavior.transcription?.transform)) { - const transform = ruleBehavior.transcription.transform; - this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight ?? 0); - } - } } export interface WebviewInitOptionSpec extends InitOptionSpec { diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index 4807e37756a..00242ce5d0f 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -1,12 +1,13 @@ -import { type Keyboard, Mock, OutputTarget } from '@keymanapp/keyboard-processor'; +import { type Keyboard, Mock, OutputTarget, Transcription } from '@keymanapp/keyboard-processor'; import { KeyboardStub } from 'keyman/engine/package-cache'; import { ContextManagerBase, ContextManagerConfiguration } from 'keyman/engine/main'; import { WebviewConfiguration } from './configuration.js'; export type OnInsertTextFunc = (deleteLeft: number, text: string, deleteRight: number) => void; -class ContextHost extends Mock { +export class ContextHost extends Mock { readonly oninserttext?: OnInsertTextFunc; + private savedState: Mock; constructor(oninserttext: OnInsertTextFunc) { super(); @@ -15,21 +16,39 @@ class ContextHost extends Mock { apply(transform: Transform): void { super.apply(transform); + this.updateHost(); + } + + updateHost(transcription?: Transcription): void { + const savedState = this.savedState; + + if(this.savedState) { + let transform = null; + + if(transcription) { + const preInput = transcription.preInput; + // If our saved state matches the `preInput` from the incoming transcription, just reuse its transform. + // Will generally not match during multitap operations, though. + if(preInput.text == savedState.text && preInput.selStart == savedState.selStart && preInput.selEnd == savedState.selEnd) { + transform = transcription.transform; + } + } + + transform ||= this.buildTransformFrom(this.savedState); - // Signal the necessary text changes to the embedding app, if it exists. - if(this.oninserttext) { - this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight); + // Signal the necessary text changes to the embedding app, if it exists. + if(this.oninserttext) { + this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight); + } } + + // Save the current context state for use in future diffs. + this.savedState = Mock.from(this); } restoreTo(original: OutputTarget): void { - const reversionTransform = original.buildTransformFrom(this); - + this.savedState = Mock.from(this); super.restoreTo(original); - - if(this.oninserttext) { - this.oninserttext(reversionTransform.deleteLeft, reversionTransform.insert, reversionTransform.deleteRight); - } } // In app/webview, apps are expected to immediately update the selection range AFTER @@ -40,7 +59,8 @@ class ContextHost extends Mock { // Our host app will not know whether or not the keyboard uses SMP chars, // and we want a consistent interface for context synchronization between // host app + app/webview KMW. - this.setSelection(this.text.kmwLength()); + this.setSelection(this.text._kmwLength()); + this.savedState = Mock.from(this); } } diff --git a/web/src/app/webview/src/keymanEngine.ts b/web/src/app/webview/src/keymanEngine.ts index 938aaf6c265..60259443afc 100644 --- a/web/src/app/webview/src/keymanEngine.ts +++ b/web/src/app/webview/src/keymanEngine.ts @@ -1,11 +1,11 @@ -import { DefaultRules, DeviceSpec } from '@keymanapp/keyboard-processor' +import { DefaultRules, DeviceSpec, RuleBehavior, isEmptyTransform } from '@keymanapp/keyboard-processor' import { KeymanEngine as KeymanEngineBase, KeyboardInterface } from 'keyman/engine/main'; import { AnchoredOSKView, ViewConfiguration, StaticActivator } from 'keyman/engine/osk'; import { getAbsoluteX, getAbsoluteY } from 'keyman/engine/dom-utils'; import { type KeyboardStub, toPrefixedKeyboardId, toUnprefixedKeyboardId } from 'keyman/engine/package-cache'; import { WebviewConfiguration, WebviewInitOptionDefaults, WebviewInitOptionSpec } from './configuration.js'; -import ContextManager from './contextManager.js'; +import ContextManager, { ContextHost } from './contextManager.js'; import PassthroughKeyboard from './passthroughKeyboard.js'; import { buildEmbeddedGestureConfig, setupEmbeddedListeners } from './oskConfiguration.js'; @@ -15,6 +15,11 @@ export default class KeymanEngine extends KeymanEngineBase { + (this.context as ContextHost).updateHost(ruleBehavior.transcription); + } + config.stubNamespacer = (stub) => { // If the package has not yet been applied as namespacing... if(stub.KP && stub.KI.indexOf(`${stub.KP}::`) == -1) { From 9bc01bee5f5796c8081570b41707539613a4c0c8 Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 15 Feb 2024 13:54:16 +0700 Subject: [PATCH 2/4] fix(ios): multitaps following a context-window slide --- .../Keyboard/InputViewController.swift | 9 ++---- .../Keyboard/KeymanWebViewController.swift | 29 +++++++++-------- .../Contents/Resources/ios-host.js | 30 ++++++------------ web/src/app/webview/src/contextManager.ts | 31 ++++++++++++++++++- 4 files changed, 56 insertions(+), 43 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 64a4a3d7548..4f4bc9e12e3 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -317,7 +317,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { let range = NSRange(location: before.unicodeScalars.count, length: 0) - self.setContextState(text: contextWindowText, range: range) + self.setContextState(text: contextWindowText, range: range, doSync: true) } updater(preCaretContext, postCaretContext) @@ -606,7 +606,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { * will be expecting SMP-aware measurements. Swift's `.unicodeScalars` * property on `String`s lines up best with this. */ - func setContextState(text: String?, range: NSRange) { + func setContextState(text: String?, range: NSRange, doSync: Bool = false) { // Check for any LTR or RTL marks at the context's start; if they exist, we should // offset the selection range. let characterOrderingChecks = [ "\u{200e}" /*LTR*/, "\u{202e}" /*RTL 1*/, "\u{200f}" /*RTL 2*/ ] @@ -626,10 +626,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { selRange = NSRange(location: selRange.location - 1, length: selRange.length) } - keymanWeb.setText(context) - if range.location != NSNotFound { - keymanWeb.setCursorRange(selRange) - } + keymanWeb.setContext(text: context, range: selRange, doSync: doSync) } func resetKeyboardState() { diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift index af7d1950396..b568fb4baff 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift @@ -223,14 +223,7 @@ extension KeymanWebViewController { setSpacebarText(userData.optSpacebarText) } - func setCursorRange(_ range: NSRange) { - if range.location != NSNotFound { - webView!.evaluateJavaScript("setCursorRange(\(range.location),\(range.length));", completionHandler: nil) - self.currentCursorRange = range - } - } - - func setText(_ text: String?) { + func setContext(text: String?, range: NSRange?, doSync: Bool = false) { var text = text ?? "" // Remove any system-added LTR/RTL marks. @@ -246,7 +239,16 @@ extension KeymanWebViewController { let jsonText = jsonString[start.. => NaN. + end = isNaN(end) ? undefined : end; + + const shouldReset = keyman.context.updateContext(text, start, end); + if(!doSync || shouldReset) { keyman.resetContext(); } - - return keyman.context.getText(); } function executePopupKey(keyID, keyText) { diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index 00242ce5d0f..1f4f09935f2 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -1,4 +1,4 @@ -import { type Keyboard, Mock, OutputTarget, Transcription } from '@keymanapp/keyboard-processor'; +import { type Keyboard, Mock, OutputTarget, Transcription, findCommonSubstringEndIndex } from '@keymanapp/keyboard-processor'; import { KeyboardStub } from 'keyman/engine/package-cache'; import { ContextManagerBase, ContextManagerConfiguration } from 'keyman/engine/main'; import { WebviewConfiguration } from './configuration.js'; @@ -51,6 +51,35 @@ export class ContextHost extends Mock { super.restoreTo(original); } + updateContext(text: string, selStart: number, selEnd: number): boolean { + let shouldResetContext = false; + if(text != this.text) { + let tempMock = new Mock(text, selStart ?? text._kmwLength(), selEnd ?? text._kmwLength()); + + let newLeft = tempMock.getTextBeforeCaret(); + let oldLeft = this.getTextBeforeCaret(); + + let unexpectedBeforeCharCount = findCommonSubstringEndIndex(newLeft, oldLeft, true) + 1; + shouldResetContext = !!unexpectedBeforeCharCount; + } + + if(shouldResetContext) { + this.text = text; + } + + if(selStart === undefined || selEnd === undefined) { + // Regardless of keyboard, we should check the SMP-aware length of the string. + // Our host app will not know whether or not the keyboard uses SMP chars, + // and we want a consistent interface for context synchronization between + // host app + app/webview KMW. + this.setSelection(this.text._kmwLength()); + } + + this.savedState = Mock.from(this); + + return shouldResetContext; + } + // In app/webview, apps are expected to immediately update the selection range AFTER // changing the context's text. setText(text: string): void { From d2719f028efe36afd5ebe000ca011cd2792bbd4c Mon Sep 17 00:00:00 2001 From: jahorton Date: Thu, 15 Feb 2024 14:22:18 +0700 Subject: [PATCH 3/4] docs(web): a few comments --- web/src/app/webview/src/contextManager.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index 1f4f09935f2..b887bac15c5 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -29,6 +29,8 @@ export class ContextHost extends Mock { const preInput = transcription.preInput; // If our saved state matches the `preInput` from the incoming transcription, just reuse its transform. // Will generally not match during multitap operations, though. + // + // Helps ensure backspaces pass through even if we don't currently have text available in context for them. if(preInput.text == savedState.text && preInput.selStart == savedState.selStart && preInput.selEnd == savedState.selEnd) { transform = transcription.transform; } From 9ab01d272bcd2b0b48c8a54b2703a5856516ea67 Mon Sep 17 00:00:00 2001 From: jahorton Date: Mon, 19 Feb 2024 08:57:46 +0700 Subject: [PATCH 4/4] chore(ios): polish for new ios-host func per review --- .../Keyman.bundle/Contents/Resources/ios-host.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js index e7c86b721fa..621e643bb94 100644 --- a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js +++ b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js @@ -276,18 +276,18 @@ function doResetContext() { keyman.resetContext(); } -function setKeymanContext(text, doSync, pos, length) { +function setKeymanContext(text, doSync, selStart, selLength) { // console.log(`setKeymanContext(${JSON.stringify(text)}, ${doSync}, ${pos}, ${length})`); if(text == undefined) { text = ''; } - var start = pos; - var end = pos + length; - // undefined + => NaN. - end = isNaN(end) ? undefined : end; + // Both pos + length are optional parameters. + // undefined + => NaN; undefined + undefined => NaN. + let selEnd = selStart + selLength; + selEnd = isNaN(selEnd) ? undefined : selEnd; - const shouldReset = keyman.context.updateContext(text, start, end); + const shouldReset = keyman.context.updateContext(text, selStart, selEnd); if(!doSync || shouldReset) { keyman.resetContext(); }