Skip to content

Commit

Permalink
Merge pull request #10728 from keymanapp/fix/ios/context-slide-into-m…
Browse files Browse the repository at this point in the history
…ultitap

fix(ios): multitap consistency after new-lines
  • Loading branch information
jahorton authored Feb 19, 2024
2 parents de1d0ce + f5990e2 commit 453d24d
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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*/ ]
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -246,7 +239,16 @@ extension KeymanWebViewController {
let jsonText = jsonString[start..<end]

self.currentText = String(jsonText)
webView!.evaluateJavaScript("setKeymanVal(\"\(jsonText)\");", completionHandler: nil)

var finalRange = range ?? self.currentCursorRange ?? nil
self.currentCursorRange = finalRange

//
if (finalRange?.location ?? NSNotFound) != NSNotFound {
webView!.evaluateJavaScript("setKeymanContext(\"\(jsonText)\", \(doSync ? "true" : "false"), \(finalRange!.location), \(finalRange!.length));", completionHandler: nil)
} else {
webView!.evaluateJavaScript("setKeymanContext(\"\(jsonText)\", \(doSync ? "true" : "false"));", completionHandler: nil)
}
} catch {
os_log("%{public}s", log: KeymanEngineLogger.engine, type: .error, error.localizedDescription)
SentryManager.capture(error.localizedDescription)
Expand Down Expand Up @@ -363,7 +365,7 @@ extension KeymanWebViewController {
if let packageID = lexicalModel.packageID {
event.extra?["package"] = packageID
}

os_log("%{public}s: %{public}s", log: KeymanEngineLogger.resources, type: .error, errorMessage, lexicalModel.id)
SentryManager.capture(event)
throw KeyboardError.fileMissing
Expand Down Expand Up @@ -572,7 +574,7 @@ extension KeymanWebViewController: WKScriptMessageHandler {
// This may need filtering for proper use with Sentry?
// Then again, if KMW is logging it... we already have to worry
// about it showing up in Web-oriented Sentry logs.

let logMessage = "KMW Log: \(message)"
os_log("%{public}s", log: KeymanEngineLogger.engine, type: .info, logMessage)
SentryManager.breadcrumb(logMessage)
Expand Down Expand Up @@ -672,10 +674,7 @@ extension KeymanWebViewController: KeymanWebDelegate {
resizeKeyboard()

// There may have been attempts to set these values before the keyboard loaded!
self.setText(self.currentText)
if let cursorRange = self.currentCursorRange {
self.setCursorRange(cursorRange)
}
self.setContext(text: self.currentText, range: self.currentCursorRange)

var newKb = Manager.shared.currentKeyboard

Expand Down
22 changes: 0 additions & 22 deletions ios/engine/KMEI/KeymanEngine/Classes/TextField.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
16 changes: 0 additions & 16 deletions ios/engine/KMEI/KeymanEngine/Classes/TextView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,33 +276,21 @@ function doResetContext() {
keyman.resetContext();
}

function setCursorRange(pos, length) {
var context = keyman.context;

//console.log('setCursorRange('+pos+', '+length+')');

var start = pos;
var end = pos + length;

if(context.selStart != start || context.selEnd != end) {
keyman.context.setSelection(start, end);
keyman.resetContext();
}
}

function setKeymanVal(text) {
//console.log('setKeymanVal('+JSON.stringify(text)+')');

function setKeymanContext(text, doSync, selStart, selLength) {
// console.log(`setKeymanContext(${JSON.stringify(text)}, ${doSync}, ${pos}, ${length})`);
if(text == undefined) {
text = '';
}

if(keyman.context.getText() != text) {
keyman.context.setText(text);
// Both pos + length are optional parameters.
// undefined + <number> => NaN; undefined + undefined => NaN.
let selEnd = selStart + selLength;
selEnd = isNaN(selEnd) ? undefined : selEnd;

const shouldReset = keyman.context.updateContext(text, selStart, selEnd);
if(!doSync || shouldReset) {
keyman.resetContext();
}

return keyman.context.getText();
}

function executePopupKey(keyID, keyText) {
Expand Down
7 changes: 0 additions & 7 deletions web/src/app/webview/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
71 changes: 61 additions & 10 deletions web/src/app/webview/src/contextManager.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { type Keyboard, Mock, OutputTarget } 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';

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();
Expand All @@ -15,21 +16,70 @@ class ContextHost extends Mock {

apply(transform: Transform): void {
super.apply(transform);
this.updateHost();
}

// Signal the necessary text changes to the embedding app, if it exists.
if(this.oninserttext) {
this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight);
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.
//
// 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;
}
}

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);
}
}

// 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);
}

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();

if(this.oninserttext) {
this.oninserttext(reversionTransform.deleteLeft, reversionTransform.insert, reversionTransform.deleteRight);
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
Expand All @@ -40,7 +90,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);
}
}

Expand Down
9 changes: 7 additions & 2 deletions web/src/app/webview/src/keymanEngine.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -15,6 +15,11 @@ export default class KeymanEngine extends KeymanEngineBase<WebviewConfiguration,
// be compiled down to an IIFE.
constructor(worker: Worker, sourceUri: string) {
const config = new WebviewConfiguration(sourceUri); // currently set to perform device auto-detect.

config.onRuleFinalization = (ruleBehavior: RuleBehavior) => {
(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) {
Expand Down

0 comments on commit 453d24d

Please sign in to comment.