From 39c0cf6c6473b601d334e9aa1a59b00ba29f3744 Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 6 Mar 2024 10:07:43 +0700 Subject: [PATCH 01/18] fix(ios): sample build script --debug detection --- ios/keyman/build.sh | 2 +- ios/samples/common.inc.sh | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ios/keyman/build.sh b/ios/keyman/build.sh index 64b2cc2c97..dbdf5b5123 100755 --- a/ios/keyman/build.sh +++ b/ios/keyman/build.sh @@ -30,7 +30,7 @@ if builder_is_debug_build; then fi builder_describe_outputs \ - build /ios/build/Build/Products/Release-iphoneos/Keyman.xcarchive + build /ios/build/Build/Products/${CONFIG}-iphoneos/Keyman.xcarchive # Base definitions (must be before do_clean call) DERIVED_DATA="$KEYMAN_ROOT/ios/build" diff --git a/ios/samples/common.inc.sh b/ios/samples/common.inc.sh index fb8d8e969c..5991c26fad 100755 --- a/ios/samples/common.inc.sh +++ b/ios/samples/common.inc.sh @@ -36,10 +36,14 @@ function execute_sample_build() { builder_parse "$@" local CONFIG=Release - if builder_is_debug_build; then + # `builder_is_debug_build` appears to fail here, while referring to the option does not? + # Perhaps it's due to being within a function? + if builder_has_option --debug; then CONFIG="Debug" fi + echo "CONFIG = ${CONFIG}" + local BUILD_FOLDER="ios/samples/$TARGET/build" builder_describe_outputs \ From 585f3b6428775f476abd847c790cf68c33a82ec4 Mon Sep 17 00:00:00 2001 From: jahorton Date: Wed, 6 Mar 2024 14:02:03 +0700 Subject: [PATCH 02/18] chore(ios): minor cleanup --- ios/samples/common.inc.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/ios/samples/common.inc.sh b/ios/samples/common.inc.sh index 5991c26fad..55a815779b 100755 --- a/ios/samples/common.inc.sh +++ b/ios/samples/common.inc.sh @@ -42,8 +42,6 @@ function execute_sample_build() { CONFIG="Debug" fi - echo "CONFIG = ${CONFIG}" - local BUILD_FOLDER="ios/samples/$TARGET/build" builder_describe_outputs \ From c99aecacde152d2e252c0cb86108ed37520dede2 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 11 Mar 2024 12:02:57 -0500 Subject: [PATCH 03/18] fix(ios): remove '--debug' from build declaration The '--debug' flag should not be specified as a parameter in the builder_describe call. --- common/web/gesture-recognizer/test.sh | 3 +-- ios/samples/common.inc.sh | 9 ++------- oem/firstvoices/ios/build.sh | 1 - 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/common/web/gesture-recognizer/test.sh b/common/web/gesture-recognizer/test.sh index 5e09082c91..4dc6251e45 100755 --- a/common/web/gesture-recognizer/test.sh +++ b/common/web/gesture-recognizer/test.sh @@ -18,8 +18,7 @@ builder_describe "Runs all tests for the gesture-recognizer module" \ "test+" \ ":headless Runs headless user tests" \ ":browser Runs browser-based user tests" \ - "--ci Uses CI-based test configurations & emits CI-friendly test reports" \ - "--debug,-d Activates developer-friendly debug mode for unit tests where applicable" + "--ci Uses CI-based test configurations & emits CI-friendly test reports" builder_parse "$@" diff --git a/ios/samples/common.inc.sh b/ios/samples/common.inc.sh index 55a815779b..cbfc4796fd 100755 --- a/ios/samples/common.inc.sh +++ b/ios/samples/common.inc.sh @@ -8,9 +8,7 @@ function do_build() { cp -Rf "$KEYMAN_ENGINE_FRAMEWORK_SRC" "$KEYMAN_ENGINE_FRAMEWORK_DST" CODE_SIGN= - # `builder_is_debug_build` appears to fail here, while referring to the option does not? - # Perhaps it's due to the main build definition being within a function? - if builder_has_option --debug; then + if builder_is_debug_build; then CODE_SIGN=CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED="NO" CODE_SIGNING_ENTITLEMENTS="" fi @@ -30,15 +28,12 @@ function execute_sample_build() { "clean" \ "configure" \ "build" \ - "--debug Avoids codesigning and adds full sourcemaps for the embedded predictive-text engine" \ "--sim-artifact+ Unused by this build at present" builder_parse "$@" local CONFIG=Release - # `builder_is_debug_build` appears to fail here, while referring to the option does not? - # Perhaps it's due to being within a function? - if builder_has_option --debug; then + if builder_is_debug_build; then CONFIG="Debug" fi diff --git a/oem/firstvoices/ios/build.sh b/oem/firstvoices/ios/build.sh index 03693589db..b8ec107774 100755 --- a/oem/firstvoices/ios/build.sh +++ b/oem/firstvoices/ios/build.sh @@ -22,7 +22,6 @@ builder_describe "Builds the $TARGET app for use on iOS devices - iPhone and iPa "clean" \ "configure" \ "build" \ - "--debug Avoids codesigning and adds full sourcemaps for the embedded predictive-text engine" \ "--sim-artifact Also outputs a simulator-friendly test artifact corresponding to the build" builder_parse "$@" From dd284665fb078acfeaf3e5f30581d001e3d9361b Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 5 Apr 2024 15:03:50 +0700 Subject: [PATCH 04/18] fix(ios): deletion of selected text --- .../KeymanEngine/Classes/Keyboard/InputViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 172385c6fc..faa7d21e61 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -351,7 +351,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { if let selected = textDocumentProxy.selectedText { if selected.count > 0 { - textDocumentProxy.deleteBackward() + textDocumentProxy.insertText(""); hasDeletedSelection = true } } From b689fc270614a56777d3957b048dd3e2ca86c583 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 10 Apr 2024 08:51:07 +0700 Subject: [PATCH 05/18] fix(ios): workaround for empty-string insert not erasing selected text --- .../Keyboard/InputViewController.swift | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index faa7d21e61..0ecb3ee9f1 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -347,18 +347,40 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { perform(#selector(self.enableInputClickSound), with: nil, afterDelay: 0.1) } - var hasDeletedSelection = false + var deleteSelection = false if let selected = textDocumentProxy.selectedText { if selected.count > 0 { - textDocumentProxy.insertText(""); - hasDeletedSelection = true + deleteSelection = true } } - if numCharsToDelete <= 0 || hasDeletedSelection { - textDocumentProxy.insertText(newText) + if deleteSelection && newText == "" { + /* + if deleteSelection && newText == "", we have a backspace on + selected text. Sadly, .insertText("")... does nothing. Why, Apple!? + + The one silver lining: Apple makes it impossible for users to select text + in a way that splits character clusters. + + So, we can just insert something that won't combine, like a ZWNJ, and then delete it. + */ + let beforeManipulation = textDocumentProxy.documentContextBeforeInput ?? "" + textDocumentProxy.insertText("\u{200c}") + textDocumentProxy.deleteBackward() + + let afterManipulation = textDocumentProxy.documentContextBeforeInput ?? "" + + // For good measure, a canary to signal if our selected-text backspace handling + // goes awry. + if beforeManipulation != afterManipulation { + os_log(.error, log: KeymanEngineLogger.engine, "Could not cleanly execute backspace for selected text") + } + sendContextUpdate() + return + } else if numCharsToDelete <= 0 || deleteSelection { + textDocumentProxy.insertText(newText) sendContextUpdate() return } From 7df368a143159ec9644fa7ef2759fb778b6a6869 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 11 Apr 2024 11:36:10 +0700 Subject: [PATCH 06/18] fix(web): now auto-scrolls if target element would be obscured by OSK on device rotation --- web/src/app/browser/src/keymanEngine.ts | 42 ++++++++++--------- .../browser/src/utils/rotationProcessor.ts | 9 ++++ 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/web/src/app/browser/src/keymanEngine.ts b/web/src/app/browser/src/keymanEngine.ts index 6b95cb49a0..fac77150c7 100644 --- a/web/src/app/browser/src/keymanEngine.ts +++ b/web/src/app/browser/src/keymanEngine.ts @@ -73,29 +73,33 @@ export default class KeymanEngine extends KeymanEngineBase).activationTrigger = e; - if(this.config.hostDevice.touchable) { - if(!e || !target || !this.osk) { - return; - } - - // Get the absolute position of the caret - const y = getAbsoluteY(e); - const t = window.pageYOffset; - let dy = y-t; - if(y >= t) { - dy -= (window.innerHeight - this.osk._Box.offsetHeight - e.offsetHeight - 2); - if(dy < 0) { - dy=0; - } - } - // Hide OSK, then scroll, then re-anchor OSK with absolute position (on end of scroll event) - if(dy != 0) { - window.scrollTo(0, dy + t); - } + if(this.config.hostDevice.touchable && target) { + this.ensureElementVisibility(e); } }); } + public ensureElementVisibility(e: HTMLElement) { + if(!e || !this.osk) { + return; + } + + // Get the absolute position of the caret + const y = getAbsoluteY(e); + const t = window.pageYOffset; + let dy = y-t; + if(y >= t) { + dy -= (window.innerHeight - this.osk._Box.offsetHeight - e.offsetHeight - 2); + if(dy < 0) { + dy=0; + } + } + // Hide OSK, then scroll, then re-anchor OSK with absolute position (on end of scroll event) + if(dy != 0) { + window.scrollTo(0, dy + t); + } + } + public get util() { return this._util; } diff --git a/web/src/app/browser/src/utils/rotationProcessor.ts b/web/src/app/browser/src/utils/rotationProcessor.ts index 1228153497..e8141d1ab1 100644 --- a/web/src/app/browser/src/utils/rotationProcessor.ts +++ b/web/src/app/browser/src/utils/rotationProcessor.ts @@ -58,6 +58,15 @@ export class RotationProcessor { window.clearInterval(this.updateTimer); this.rotState = null; } + + const target = this.keyman.contextManager.activeTarget; + if(target) { + // This seems to help with scrolling accuracy in iOS Safari; + // the scroll tends to consistently go too far without it. + window.setTimeout(() => { + this.keyman.ensureElementVisibility(target.getElement()); + }, 0); + } } // Used by both Android and iOS. From d6df7c29f4e1920df58fbfbef66977055c9e103d Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 12 Apr 2024 10:50:06 +0700 Subject: [PATCH 07/18] fix(ios): better delete pattern --- .../Keyboard/InputViewController.swift | 89 ++++++++++++------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 0ecb3ee9f1..0662c55855 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -333,6 +333,60 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { } } } + + func deleteSelection() -> Bool { + if let selected = textDocumentProxy.selectedText, selected.count > 0 { + /* + Since we're doing some funky text manipulation, it's best to add + a "canary" check in case something does go awry with it in + the future. + */ + let beforeManipulation = textDocumentProxy.documentContextBeforeInput ?? "" + + /* + Apple has some special nuances to its text deletion that our internal + Web engine does not emulate. + + .deleteBackward() behaviors: + - If there is selected text immediately following a space (U+0020), + it will delete that space IN ADDITION to the selected text. + - If there is selected text that starts mid-word, it will NOT delete + the preceding character. + + Compare to Web: Web states an exact number of characters to delete + before the start of the currently-selected range... and it does this + completely unaware of the nuances listed above for .deleteBackward(). + Keyman keyboard rules are likewise unaware of Apple's nuances. + + In order to maintain proper synchronization between app context and + internal Web-engine context, we need to force selected-text deletion + to NEVER delete preceding spaces. Any attempts to adjust and include + the aforementioned nuance will need considerable design work to "get + right" due to the risk for adverse affects with Keyman keyboard rules. + + .insertText() is great for this... when the string isn't empty. If + it is, well, "sorry, out of luck." That said, we can just insert + something that won't combine, like a ZWNJ, and then delete it. + + The silver lining: Apple makes it impossible for users to select text + in a way that splits character clusters. This implies that it's + impossible for an inserted ZWNJ to combine with existing context, + making this operation safe. + */ + textDocumentProxy.insertText("\u{200c}") + textDocumentProxy.deleteBackward() + + let afterManipulation = textDocumentProxy.documentContextBeforeInput ?? "" + + // And now to finish our 'canary' check. + if beforeManipulation != afterManipulation { + os_log(.error, log: KeymanEngineLogger.engine, "Could not cleanly execute backspace for selected text") + } + + return true + } + return false + } func insertText(_ keymanWeb: KeymanWebViewController, numCharsToDelete: Int, newText: String) { if keymanWeb.isSubKeysMenuVisible { @@ -347,39 +401,10 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { perform(#selector(self.enableInputClickSound), with: nil, afterDelay: 0.1) } - var deleteSelection = false + // `true` if there was selected text to be deleted + let deletedSelection = self.deleteSelection() - if let selected = textDocumentProxy.selectedText { - if selected.count > 0 { - deleteSelection = true - } - } - - if deleteSelection && newText == "" { - /* - if deleteSelection && newText == "", we have a backspace on - selected text. Sadly, .insertText("")... does nothing. Why, Apple!? - - The one silver lining: Apple makes it impossible for users to select text - in a way that splits character clusters. - - So, we can just insert something that won't combine, like a ZWNJ, and then delete it. - */ - let beforeManipulation = textDocumentProxy.documentContextBeforeInput ?? "" - - textDocumentProxy.insertText("\u{200c}") - textDocumentProxy.deleteBackward() - - let afterManipulation = textDocumentProxy.documentContextBeforeInput ?? "" - - // For good measure, a canary to signal if our selected-text backspace handling - // goes awry. - if beforeManipulation != afterManipulation { - os_log(.error, log: KeymanEngineLogger.engine, "Could not cleanly execute backspace for selected text") - } - sendContextUpdate() - return - } else if numCharsToDelete <= 0 || deleteSelection { + if numCharsToDelete <= 0 || deletedSelection { textDocumentProxy.insertText(newText) sendContextUpdate() return From c2ba4327bba026a1d38c183fb315201f93052efd Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 12 Apr 2024 08:04:50 +0700 Subject: [PATCH 08/18] chore(android): sets internal WebView inspectable --- .../KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java index a356a20ad5..0858ce25e6 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java @@ -232,11 +232,11 @@ final class KMKeyboard extends WebView { getSettings().setUseWideViewPort(true); getSettings().setLoadWithOverviewMode(true); - if (0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE)) { + // if (0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE)) { // Enable debugging of WebView via adb. Not used during unit tests // Refer: https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging setWebContentsDebuggingEnabled(true); - } + // } setWebChromeClient(new WebChromeClient() { public boolean onConsoleMessage(ConsoleMessage cm) { String msg = KMString.format("KMW JS Log: Line %d, %s:%s", cm.lineNumber(), cm.sourceId(), cm.message()); From 6f3c5e053db6570f82132ce275e46c7fb1e7c17d Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 12 Apr 2024 08:21:14 +0700 Subject: [PATCH 09/18] change(android): Android app for non-stable version to enable inspectability --- .../src/main/java/com/keyman/engine/KMKeyboard.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java index 0858ce25e6..a1bec70f20 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java @@ -232,11 +232,17 @@ final class KMKeyboard extends WebView { getSettings().setUseWideViewPort(true); getSettings().setLoadWithOverviewMode(true); - // if (0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE)) { + + // When `.isTestMode() == true`, the setWebContentsDebuggingEnabled method is not available + // and thus will trigger unit-test failures. + if (!KMManager.isTestMode() && ( + 0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) + || KMManager.getTier(null) != KMManager.Tier.STABLE + )) { // Enable debugging of WebView via adb. Not used during unit tests // Refer: https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging setWebContentsDebuggingEnabled(true); - // } + } setWebChromeClient(new WebChromeClient() { public boolean onConsoleMessage(ConsoleMessage cm) { String msg = KMString.format("KMW JS Log: Line %d, %s:%s", cm.lineNumber(), cm.sourceId(), cm.message()); From 0c8f520e2aefada2958704bf58edc4ee4c9ead55 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Apr 2024 10:17:28 +0700 Subject: [PATCH 10/18] fix(web): prevents selection-clear for pure layer-switching multitaps --- .../input-processor/src/text/inputProcessor.ts | 16 ++++++++++++++-- .../keyboard-processor/src/text/outputTarget.ts | 11 +++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/common/web/input-processor/src/text/inputProcessor.ts b/common/web/input-processor/src/text/inputProcessor.ts index 76d8b288f4..55f7a1e8b6 100644 --- a/common/web/input-processor/src/text/inputProcessor.ts +++ b/common/web/input-processor/src/text/inputProcessor.ts @@ -107,8 +107,20 @@ export default class InputProcessor { if(keyEvent.baseTranscriptionToken) { const transcription = this.contextCache.get(keyEvent.baseTranscriptionToken); if(transcription) { - // Restores full context, including deadkeys in their exact pre-keystroke state. - outputTarget.restoreTo(transcription.preInput); + // Has there been a context change at any point during the multitap? If so, we need + // to revert it. If not, we assume it's a layer-change multitap, in which case + // no such reset is needed. + if(!isEmptyTransform(transcription.transform) || !transcription.preInput.isEqual(Mock.from(outputTarget))) { + // Restores full context, including deadkeys in their exact pre-keystroke state. + outputTarget.restoreTo(transcription.preInput); + } + /* + else: + 1. We don't need to restore the original context, as it's already + in-place. + 2. Restoring anyway would obliterate any selected text, which is bad + if this is a purely-layer-switching multitap. (#11230) + */ } else { console.warn('The base context for the multitap could not be found'); } diff --git a/common/web/keyboard-processor/src/text/outputTarget.ts b/common/web/keyboard-processor/src/text/outputTarget.ts index ef3a3d74e3..2f83f17df4 100644 --- a/common/web/keyboard-processor/src/text/outputTarget.ts +++ b/common/web/keyboard-processor/src/text/outputTarget.ts @@ -446,6 +446,17 @@ export class Mock extends OutputTarget { this.text = this.getTextBeforeCaret() + s; } + /** + * Indicates if this Mock represents an identical context to that of another Mock. + * + * Does not currently validate a match for deadkeys. + * @param other + * @returns + */ + isEqual(other: Mock) { + return this.text == other.text && this.selStart == other.selStart && this.selEnd == other.selEnd; + } + doInputEvent() { // Mock isn't backed by an element, so it won't have any event listeners. } From fb127e761b77708d9dfc59b92a8c93d50c4e0de0 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Apr 2024 10:33:16 +0700 Subject: [PATCH 11/18] fix(web): more robustness in case of deadkeys --- .../keyboard-processor/src/text/deadkeys.ts | 22 +++++++++++++++++++ .../src/text/outputTarget.ts | 7 +++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/common/web/keyboard-processor/src/text/deadkeys.ts b/common/web/keyboard-processor/src/text/deadkeys.ts index e5b32a788f..31bfa1100d 100644 --- a/common/web/keyboard-processor/src/text/deadkeys.ts +++ b/common/web/keyboard-processor/src/text/deadkeys.ts @@ -38,6 +38,10 @@ export class Deadkey { return dk; } + equal(other: Deadkey) { + return this.d == other.d && this.p == other.d && this.o == other.o; + } + /** * Sorts the deadkeys in reverse order. */ @@ -151,6 +155,24 @@ export class DeadkeyTracker { } } + equal(other: DeadkeyTracker) { + if(this.dks.length != other.dks.length) { + return false; + } + + const otherDks = other.dks; + const matchedDks: Deadkey[] = []; + + for(let dk of this.dks) { + const match = otherDks.find((otherDk) => dk.equal(otherDk)); + if(!match) { + return false; + } + } + + return matchedDks.length == otherDks.length; + } + count(): number { return this.dks.length; } diff --git a/common/web/keyboard-processor/src/text/outputTarget.ts b/common/web/keyboard-processor/src/text/outputTarget.ts index 2f83f17df4..c7f42d8ee5 100644 --- a/common/web/keyboard-processor/src/text/outputTarget.ts +++ b/common/web/keyboard-processor/src/text/outputTarget.ts @@ -448,13 +448,14 @@ export class Mock extends OutputTarget { /** * Indicates if this Mock represents an identical context to that of another Mock. - * - * Does not currently validate a match for deadkeys. * @param other * @returns */ isEqual(other: Mock) { - return this.text == other.text && this.selStart == other.selStart && this.selEnd == other.selEnd; + return this.text == other.text + && this.selStart == other.selStart + && this.selEnd == other.selEnd + && this.deadkeys().equal(other.deadkeys()); } doInputEvent() { From 9eca52c203c8084d913488982bef0023c5f93a0d Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 17 Apr 2024 11:19:27 +0700 Subject: [PATCH 12/18] chore(android): applies suggestions per review Co-authored-by: Marc Durdin --- .../KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java index a1bec70f20..78c31f3275 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java @@ -236,8 +236,8 @@ final class KMKeyboard extends WebView { // When `.isTestMode() == true`, the setWebContentsDebuggingEnabled method is not available // and thus will trigger unit-test failures. if (!KMManager.isTestMode() && ( - 0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) - || KMManager.getTier(null) != KMManager.Tier.STABLE + (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0 || + KMManager.getTier(null) != KMManager.Tier.STABLE )) { // Enable debugging of WebView via adb. Not used during unit tests // Refer: https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging From eead04351fde5398d2e90b0003bd76b71629e3f9 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Apr 2024 12:20:43 +0700 Subject: [PATCH 13/18] chore(web): simple layout reflow polish --- web/src/engine/main/src/keymanEngine.ts | 35 ++++++++----------- .../osk/src/keyboard-layout/oskBaseKey.ts | 2 +- .../engine/osk/src/keyboard-layout/oskKey.ts | 4 +-- web/src/engine/osk/src/visualKeyboard.ts | 14 ++------ 4 files changed, 21 insertions(+), 34 deletions(-) diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index e6dfb06314..ea7254ba5a 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -144,7 +144,7 @@ export default class KeymanEngine< this.osk.startHide(false); } - const earlyBatchClosure = () => { + const prepareKeyboardSwap = () => { this.refreshModel(); // Triggers context resets that can trigger layout stuff. // It's not the final such context-reset, though. @@ -156,17 +156,6 @@ export default class KeymanEngine< }); } - /* - Needed to ensure the correct layer is displayed AND that deadkeys from - the old keyboard have been wiped. - - Needs to be after the OSK has loaded for the keyboard in case the default - layer should be something other than "default" for the current context. - */ - const doContextReset = () => { - this.contextManager.resetContext(); - } - /* This pattern is designed to minimize layout reflow during the keyboard-swap process. The 'default' layer is loaded by default, but some keyboards will start on different @@ -177,16 +166,24 @@ export default class KeymanEngine< */ if(this.osk) { this.osk.batchLayoutAfter(() => { - earlyBatchClosure(); + prepareKeyboardSwap(); this.osk.activeKeyboard = kbd; // Note: when embedded within the mobile apps, the keyboard will still be visible // at this time. - doContextReset(); + + /* + Needed to ensure the correct layer is displayed AND that deadkeys from + the old keyboard have been wiped. + + Needs to be after the OSK has loaded for the keyboard in case the default + layer should be something other than "default" for the current context. + */ + this.contextManager.resetContext(); this.osk.present(); }); } else { - earlyBatchClosure(); - doContextReset(); + prepareKeyboardSwap(); + this.contextManager.resetContext(); } }); @@ -242,14 +239,12 @@ export default class KeymanEngine< resetContext: (target) => { // Could reset the target's deadkeys here, but it's really more of a 'core' task. // So we delegate that to keyboard-processor. - const doReset = () => this.core.resetContext(target); - if(this.osk) { this.osk.batchLayoutAfter(() => { - doReset(); + this.core.resetContext(target); }) } else { - doReset(); + this.core.resetContext(target); } }, predictionContext: new PredictionContext(this.core.languageProcessor, this.core.keyboardProcessor), diff --git a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts index 2a273c17ab..f535ce2944 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts @@ -203,7 +203,7 @@ export default class OSKBaseKey extends OSKKey { // part 2: key internals - these do depend on recalculating internal layout. // Ideally, the rest would be in yet another calculation layer... need to figure out a good design for this. - keyTextClosure(); // we're already in that phase, so go ahead and run it. + keyTextClosure?.(); // we're already in that phase, so go ahead and run it. const emFont = layoutParams.baseEmFontSize; // Rescale keycap labels on small phones diff --git a/web/src/engine/osk/src/keyboard-layout/oskKey.ts b/web/src/engine/osk/src/keyboard-layout/oskKey.ts index 34ee93e300..4502e6bb7a 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskKey.ts @@ -314,7 +314,7 @@ export default abstract class OSKKey { public refreshLayout(layoutParams: KeyLayoutParams) { // Avoid doing any font-size related calculations if there's no text to display. if(this.spec.sp == ButtonClasses.spacer || this.spec.sp == ButtonClasses.blank) { - return () => {}; + return null; } // Attempt to detect static but key-specific style properties if they haven't yet @@ -324,7 +324,7 @@ export default abstract class OSKKey { // Abort if the element is not currently in the DOM; we can't get any info this way. if(!lblStyle.fontFamily) { - return () => {}; + return null; } this._fontFamily = lblStyle.fontFamily; diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 1b61a722c4..4a07df109b 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -231,20 +231,12 @@ export default class VisualKeyboard extends EventEmitter implements Ke activeGestures: GestureHandler[] = []; activeModipress: Modipress = null; - private _deferLayout: boolean; + public deferLayout: boolean; // The keyboard object corresponding to this VisualKeyboard. public readonly layoutKeyboard: Keyboard; public readonly layoutKeyboardProperties: KeyboardProperties; - get deferLayout(): boolean { - return this._deferLayout; - } - - set deferLayout(value: boolean) { - this._deferLayout = value; - } - get layerId(): string { return this.layerGroup?.activeLayerId ?? 'default'; } @@ -262,7 +254,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke } } - if(changedLayer && !this._deferLayout) { + if(changedLayer && !this.deferLayout) { this.updateState(); // We changed the active layer, but not any layout property of the keyboard as a whole. this.layerGroup.refreshLayout(this.constructLayoutParams()); @@ -1215,7 +1207,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke * when needed. */ refreshLayout() { - if(this._deferLayout) { + if(this.deferLayout) { return; } From 1963ecfa11630232ad3950ec28c4b648d38c197d Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Wed, 17 Apr 2024 12:58:05 +0700 Subject: [PATCH 14/18] change(web): drops need for closures to optimize layout-reflow --- .../osk/src/keyboard-layout/oskBaseKey.ts | 23 ++--- .../engine/osk/src/keyboard-layout/oskKey.ts | 25 +++-- .../osk/src/keyboard-layout/oskLayer.ts | 48 ++++------ .../engine/osk/src/keyboard-layout/oskRow.ts | 94 +++++++++++-------- 4 files changed, 98 insertions(+), 92 deletions(-) diff --git a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts index f535ce2944..519c198306 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts @@ -197,22 +197,15 @@ export default class OSKBaseKey extends OSKKey { } public refreshLayout(layoutParams: KeyLayoutParams) { - const keyTextClosure = super.refreshLayout(layoutParams); // key labels in particular. + super.refreshLayout(layoutParams); // key labels in particular. - return () => { - // part 2: key internals - these do depend on recalculating internal layout. - - // Ideally, the rest would be in yet another calculation layer... need to figure out a good design for this. - keyTextClosure?.(); // we're already in that phase, so go ahead and run it. - - const emFont = layoutParams.baseEmFontSize; - // Rescale keycap labels on small phones - if(emFont.val < 12) { - this.capLabel.style.fontSize = '6px'; - } else { - // The default value set within kmwosk.css. - this.capLabel.style.fontSize = ParsedLengthStyle.forScalar(0.5).styleString; - } + const emFont = layoutParams.baseEmFontSize; + // Rescale keycap labels on small phones + if(emFont.val < 12) { + this.capLabel.style.fontSize = '6px'; + } else { + // The default value set within kmwosk.css. + this.capLabel.style.fontSize = ParsedLengthStyle.forScalar(0.5).styleString; } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskKey.ts b/web/src/engine/osk/src/keyboard-layout/oskKey.ts index 4502e6bb7a..378ae01b11 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskKey.ts @@ -311,7 +311,14 @@ export default abstract class OSKKey { this.label.style.fontSize = ''; } - public refreshLayout(layoutParams: KeyLayoutParams) { + /** + * Any style-caching behavior needed for use in layout manipulation should be + * computed within this method, not within refreshLayout. This is to prevent + * unnecessary layout-reflow. + * @param layoutParams + * @returns + */ + public detectStyles(layoutParams: KeyLayoutParams) { // Avoid doing any font-size related calculations if there's no text to display. if(this.spec.sp == ButtonClasses.spacer || this.spec.sp == ButtonClasses.blank) { return null; @@ -341,26 +348,26 @@ export default abstract class OSKKey { this._fontSize = ParsedLengthStyle.forScalar(localFontScaling); } } + } + // Avoid any references to getComputedStyle, offset_, or other layout-reflow + // dependent values. Refer to https://gist.github.com/paulirish/5d52fb081b3570c81e3a. + public refreshLayout(layoutParams: KeyLayoutParams) { // space bar may not define the text span! if(this.label) { if(!this.label.classList.contains('kmw-spacebar-caption')) { // Do not use `this.keyText` - it holds *___* codes for special keys, not the actual glyph! const keyCapText = this.label.textContent; const fontSize = this.getIdealFontSize(keyCapText, layoutParams); - return () => { - this.label.style.fontSize = fontSize.styleString; - }; + this.label.style.fontSize = fontSize.styleString; } else { // Spacebar text, on the other hand, is available via this.keyText. // Using this field helps prevent layout reflow during updates. const fontSize = this.getIdealFontSize(this.keyText, layoutParams); - return () => { - // Since the kmw-spacebar-caption version uses !important, we must specify - // it directly on the element too; otherwise, scaling gets ignored. - this.label.style.setProperty("font-size", fontSize.styleString, "important"); - }; + // Since the kmw-spacebar-caption version uses !important, we must specify + // it directly on the element too; otherwise, scaling gets ignored. + this.label.style.setProperty("font-size", fontSize.styleString, "important"); } } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskLayer.ts b/web/src/engine/osk/src/keyboard-layout/oskLayer.ts index ee84679a21..9a1797fedd 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskLayer.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskLayer.ts @@ -126,29 +126,35 @@ export default class OSKLayer { **/ showLanguage(displayName: string) { if(!this.spaceBarKey) { - return () => {}; + return; } try { const spacebarLabel = this.spaceBarKey.label; - // The key can read the text from here during the display update without us - // needing to trigger a reflow by running the closure below early. + // The key can read the text from here during the display update without + // triggering a reflow. this.spaceBarKey.spec.text = displayName; - return () => { - // It sounds redundant, but this dramatically cuts down on browser DOM processing; - // but sometimes innerText is reported empty when it actually isn't, so set it - // anyway in that case (Safari, iOS 14.4) - if (spacebarLabel.innerText != displayName || displayName == '') { - spacebarLabel.innerText = displayName; - } + // It sounds redundant, but this dramatically cuts down on browser DOM processing; + // but sometimes innerText is reported empty when it actually isn't, so set it + // anyway in that case (Safari, iOS 14.4) + if (spacebarLabel.innerText != displayName || displayName == '') { + spacebarLabel.innerText = displayName; } } catch (ex) { } } public refreshLayout(layoutParams: LayerLayoutParams) { + // Do all layout-reflow / style-refresh dependent precalculations here, + // before we perform any DOM manipulation. + this.rows.forEach((row) => row.detectStyles(layoutParams)); + + // Hereafter, avoid any references to getComputedStyle, offset_, or other + // layout-reflow dependent values. Refer to + // https://gist.github.com/paulirish/5d52fb081b3570c81e3a. + // Check the heights of each row, in case different layers have different row counts. const layerHeight = layoutParams.keyboardHeight; const nRows = this.rows.length; @@ -159,10 +165,9 @@ export default class OSKLayer { this.element.style.height=(layerHeight)+'px'; } - const spacebarTextClosure = this.showLanguage(layoutParams.spacebarText); + this.showLanguage(layoutParams.spacebarText); // Update row layout properties - const rowClosures: (() => void)[] = []; for(let nRow=0; nRow { - oldRowClosure(); - oskRow.element.style.bottom = '1px'; - }; + oskRow.element.style.bottom = '1px'; } - rowClosures.push(rowClosure); } - const rowKeyClosures: (() => void)[] = []; for(const row of this.rows) { - const batchedUpdates = row.refreshKeyLayouts(layoutParams); - rowKeyClosures.push(batchedUpdates); + row.refreshKeyLayouts(layoutParams); } - - // After row layout properties have been updated, _then_ update key internals. - // Doing this separately like this helps to reduce layout reflow. - spacebarTextClosure(); - rowClosures.forEach((closure) => closure()); - rowKeyClosures.forEach((closure) => closure()); } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskRow.ts b/web/src/engine/osk/src/keyboard-layout/oskRow.ts index 777b8e7658..8012e8438a 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskRow.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskRow.ts @@ -3,7 +3,7 @@ import { ActiveKey, ActiveLayer, ActiveRow } from '@keymanapp/keyboard-processor import OSKBaseKey from './oskBaseKey.js'; import { ParsedLengthStyle } from '../lengthStyle.js'; import VisualKeyboard from '../visualKeyboard.js'; -import { KeyLayoutParams } from './oskKey.js'; +import OSKKey, { KeyLayoutParams } from './oskKey.js'; import { LayerLayoutParams } from './oskLayer.js'; /* @@ -66,42 +66,68 @@ export default class OSKRow { } } + // Avoid any references to getComputedStyle, offset_, or other layout-reflow + // dependent values. Refer to https://gist.github.com/paulirish/5d52fb081b3570c81e3a. public refreshLayout(layoutParams: LayerLayoutParams) { const rs = this.element.style; const rowHeight = layoutParams.heightStyle.scaledBy(this.heightFraction); - const executeRowStyleUpdates = () => { - rs.maxHeight=rs.lineHeight=rs.height=rowHeight.styleString; - } + rs.maxHeight=rs.lineHeight=rs.height=rowHeight.styleString; const keyHeightBase = layoutParams.heightStyle.absolute ? rowHeight : ParsedLengthStyle.forScalar(1); const padTop = keyHeightBase.scaledBy(KEY_BTN_Y_PAD_RATIO / 2); const keyHeight = keyHeightBase.scaledBy(1 - KEY_BTN_Y_PAD_RATIO); // Update all key-square layouts. - const keyStyleUpdates = this.keys.map((key) => { - return () => { - const keySquare = key.square; - const keyElement = key.btn; + this.keys.forEach((key) => { + const keySquare = key.square; + const keyElement = key.btn; - // Set the kmw-key-square position - const kss = keySquare.style; - kss.height=kss.minHeight=keyHeightBase.styleString; + // Set the kmw-key-square position + const kss = keySquare.style; + kss.height=kss.minHeight=keyHeightBase.styleString; - const kes = keyElement.style; - kes.top = padTop.styleString; - kes.height=kes.lineHeight=kes.minHeight=keyHeight.styleString; - } - }) - - return () => { - executeRowStyleUpdates(); - keyStyleUpdates.forEach((closure) => closure()); - } + const kes = keyElement.style; + kes.top = padTop.styleString; + kes.height=kes.lineHeight=kes.minHeight=keyHeight.styleString; + }); } + private buildKeyLayout(layoutParams: LayerLayoutParams, key: OSKKey) { + // Calculate changes to be made... + const keyWidth = layoutParams.widthStyle.scaledBy(key.spec.proportionalWidth); + + // We maintain key-btn padding within the key-square - the latter `scaledBy` + // adjusts for that, providing the final key-btn height. + const keyHeight = layoutParams.heightStyle.scaledBy(this.heightFraction).scaledBy(1 - KEY_BTN_Y_PAD_RATIO); + + const keyStyle: KeyLayoutParams = { + keyWidth: keyWidth.val * (keyWidth.absolute ? 1 : layoutParams.keyboardWidth), + keyHeight: keyHeight.val * (keyHeight.absolute ? 1 : layoutParams.keyboardHeight), + baseEmFontSize: layoutParams.baseEmFontSize, + layoutFontSize: layoutParams.layoutFontSize + }; + + return keyStyle; + } + + /** + * Any style-caching behavior needed for use in layout manipulation should be + * computed within this method, not within refreshLayout. This is to prevent + * unnecessary layout-reflow. + * @param layoutParams + * @returns + */ + public detectStyles(layoutParams: LayerLayoutParams) { + this.keys.forEach((key) => { + key.detectStyles(this.buildKeyLayout(layoutParams, key)); + }); + } + + // Avoid any references to getComputedStyle, offset_, or other layout-reflow + // dependent values. Refer to https://gist.github.com/paulirish/5d52fb081b3570c81e3a. public refreshKeyLayouts(layoutParams: LayerLayoutParams) { - const updateClosures = this.keys.map((key) => { + this.keys.forEach((key) => { // Calculate changes to be made... const keyElement = key.btn; @@ -118,26 +144,14 @@ export default class OSKRow { // Match the row height (if fixed-height) or use full row height (if percent-based) const styleHeight = heightStyle.absolute ? keyHeight.styleString : '100%'; - const keyStyle: KeyLayoutParams = { - keyWidth: keyWidth.val * (keyWidth.absolute ? 1 : layoutParams.keyboardWidth), - keyHeight: keyHeight.val * (heightStyle.absolute ? 1 : layoutParams.keyboardHeight), - baseEmFontSize: layoutParams.baseEmFontSize, - layoutFontSize: layoutParams.layoutFontSize - }; - //return keyElement.key ? keyElement.key.refreshLayout(keyStyle) : () => {}; - const keyFontClosure = keyElement.key ? keyElement.key.refreshLayout(keyStyle) : () => {}; + const keyStyle: KeyLayoutParams = this.buildKeyLayout(layoutParams, key); + keyElement.key?.refreshLayout(keyStyle); - // And queue them to be run in a single batch later. This helps us avoid layout reflow thrashing. - return () => { - key.square.style.width = keyWidth.styleString; - key.square.style.marginLeft = keyPad.styleString; + key.square.style.width = keyWidth.styleString; + key.square.style.marginLeft = keyPad.styleString; - key.btn.style.width = widthStyle.absolute ? keyWidth.styleString : '100%'; - key.square.style.height = styleHeight; - keyFontClosure(); - } + key.btn.style.width = widthStyle.absolute ? keyWidth.styleString : '100%'; + key.square.style.height = styleHeight; }); - - return () => updateClosures.forEach((closure) => closure()); } } \ No newline at end of file From 650e86d5ac79ef256f4d8efbfa1f2712e6ad766c Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Wed, 17 Apr 2024 15:03:36 +0700 Subject: [PATCH 15/18] docs(ios): applies comment suggestions from code review --- .../Keyboard/InputViewController.swift | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 0662c55855..5fec149e1b 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -344,34 +344,29 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { let beforeManipulation = textDocumentProxy.documentContextBeforeInput ?? "" /* - Apple has some special nuances to its text deletion that our internal - Web engine does not emulate. + We have a problem to resolve here: we cannot simply delete the selection + with either .deleteBackward() or .insertText(""): - .deleteBackward() behaviors: - If there is selected text immediately following a space (U+0020), - it will delete that space IN ADDITION to the selected text. - - If there is selected text that starts mid-word, it will NOT delete - the preceding character. + .deleteBackward() will delete that space IN ADDITION to the selected text. + - Unlike .insertText("-any-string-here"), .insertText("") does nothing; + it does not replace the selection with the new string. - Compare to Web: Web states an exact number of characters to delete - before the start of the currently-selected range... and it does this - completely unaware of the nuances listed above for .deleteBackward(). - Keyman keyboard rules are likewise unaware of Apple's nuances. + Our policy (#9073) on handling the backspace key when there is a + text selection is to just delete the selection. We have to override + the special case of space being deleted by .deleteBackward() ourselves. + Additionally, the internal Web engine cannot anticipate the special + case and requires precise and consistent backspace handling in line + with our policy in order to keep the context on both sides synchronized. - In order to maintain proper synchronization between app context and - internal Web-engine context, we need to force selected-text deletion - to NEVER delete preceding spaces. Any attempts to adjust and include - the aforementioned nuance will need considerable design work to "get - right" due to the risk for adverse affects with Keyman keyboard rules. + As .insertText() does not delete the selection if the string to + be inserted is empty, we insert something that won't combine, + like a ZWNJ, and then delete it. - .insertText() is great for this... when the string isn't empty. If - it is, well, "sorry, out of luck." That said, we can just insert - something that won't combine, like a ZWNJ, and then delete it. - - The silver lining: Apple makes it impossible for users to select text - in a way that splits character clusters. This implies that it's - impossible for an inserted ZWNJ to combine with existing context, - making this operation safe. + iOS does not allow users to select text in a way that splits + character clusters. This implies that it's impossible for an + inserted ZWNJ to combine with existing context, making this + operation safe. */ textDocumentProxy.insertText("\u{200c}") textDocumentProxy.deleteBackward() @@ -404,6 +399,8 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { // `true` if there was selected text to be deleted let deletedSelection = self.deleteSelection() + // If text was selected, we generally act as if the context is nil - no back + // deletions allowed, so we skip that section. if numCharsToDelete <= 0 || deletedSelection { textDocumentProxy.insertText(newText) sendContextUpdate() From 65abf68d3e6a55b444277660d97b36c28977a941 Mon Sep 17 00:00:00 2001 From: Keyman Build Agent Date: Wed, 17 Apr 2024 14:15:00 -0400 Subject: [PATCH 16/18] auto: increment beta version to 17.0.310 --- HISTORY.md | 8 ++++++++ VERSION.md | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index 6b228ac678..ce907a869f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,13 @@ # Keyman Version History +## 17.0.309 beta 2024-04-17 + +* fix(ios): sample build script --debug detection (#10953) +* chore(web): simple layout reflow polish 🪠 (#11237) +* chore(android): enables debugging and inspection of mobile app internal webviews (#11215) +* (#11232) +* (#11238) + ## 17.0.308 beta 2024-04-13 * chore(developer): use keyboard3 tag rather than DTD to identify LDML keyboard xml files (#11214) diff --git a/VERSION.md b/VERSION.md index 2c9d7f85bd..cd50a0da6f 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -17.0.309 \ No newline at end of file +17.0.310 \ No newline at end of file From db514e0dc5e3e349a8bd113bb614e64d472a97cf Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 18 Apr 2024 09:05:30 +0700 Subject: [PATCH 17/18] fix(common): update emoji stripping for Unicode 15.1 Fixes #11220. Replaces the sed script for matching emoji in ios prepRelease with a tiny node project that uses emoji-regex package, to reduce future maintenance. Also strips lingering emoji and :xxx: emoji shortcut strings from HISTORY.md. Note that :xxx: are not stripped programatically at this time. --- HISTORY.md | 74 ++++++++++---------- ios/tools/prepRelease.sh | 5 +- package-lock.json | 29 ++++++++ package.json | 1 + resources/build/version/package.json | 1 + resources/build/version/src/reportHistory.ts | 7 +- resources/tools/strip-emoji/index.js | 18 +++++ resources/tools/strip-emoji/package.json | 14 ++++ 8 files changed, 106 insertions(+), 43 deletions(-) create mode 100644 resources/tools/strip-emoji/index.js create mode 100644 resources/tools/strip-emoji/package.json diff --git a/HISTORY.md b/HISTORY.md index 68fa259f91..f40c4195a5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,9 +3,9 @@ ## 17.0.307 beta 2024-04-12 * fix(common): specify title explicitly when opening PR with hub (#11173) -* refactor(web): better centralizes OSK layout internals to prepare for optimization efforts 🪠 (#11176) -* feat(web): VisualKeyboard layout-reflow optimization 🪠 (#11177) -* change(web): OSK optimization, improved responsiveness 🪠 (#11140) +* refactor(web): better centralizes OSK layout internals to prepare for optimization efforts (#11176) +* feat(web): VisualKeyboard layout-reflow optimization (#11177) +* change(web): OSK optimization, improved responsiveness (#11140) * (#11216) ## 17.0.306 beta 2024-04-11 @@ -16,9 +16,9 @@ ## 17.0.305 beta 2024-04-10 * (#11169) -* change(web): merges split async method in gesture engine 🪠 (#11142) +* change(web): merges split async method in gesture engine (#11142) * fix(web): blocks nextLayer for keys quickly typed when multitapping to new layer when final tap is held (#11189) -* refactor(web): OSK spacebar-label updates now managed by layer object 🪠 (#11175) +* refactor(web): OSK spacebar-label updates now managed by layer object (#11175) ## 17.0.304 beta 2024-04-09 @@ -38,14 +38,14 @@ * feat(core): support modifiers=other (#11118) * chore(core): dx better err message on embedded test vkeys (#11119) -* fix(web): key preview stickiness 🪠 (#10778) -* fix(web): early gesture-match abort when unable to extend existing gestures 🪠 (#10836) -* fix(web): infinite model-match replacement looping 🪠 (#10838) -* fix(web): proper gesture-match sequencing 🪠 (#10840) -* change(web): input-event sequentialization 🪠 (#10843) -* fix(web): proper linkage of sources to events 🪠 (#10960) +* fix(web): key preview stickiness (#10778) +* fix(web): early gesture-match abort when unable to extend existing gestures (#10836) +* fix(web): infinite model-match replacement looping (#10838) +* fix(web): proper gesture-match sequencing (#10840) +* change(web): input-event sequentialization (#10843) +* fix(web): proper linkage of sources to events (#10960) * fix(developer): handle buffer boundaries in four cases (#11137) -* chore(linux): Build packages for next Ubuntu version separately :cherries: (#11153) +* chore(linux): Build packages for next Ubuntu version separately (#11153) * fix(common): upgrade sentry-cli to 2.31.0 (#11151) * fix(android/app): Track previous device orientation for SystemKeyboard (#11134) * (#11129) @@ -2024,8 +2024,8 @@ * fix(developer): package editor no longer loses RTL flag for LMs (#8607) * chore(common): use mac /usr/bin/stat rather than homebrew version (#8925) * chore(ios): replace fv cert (#8923) -* fix(core): Fix compilation if hotdoc is installed :cherries: (#8929) -* chore(linux): Move some files to keyman-config :cherries: (#8930) +* fix(core): Fix compilation if hotdoc is installed (#8929) +* chore(linux): Move some files to keyman-config (#8930) * fix(core): Fix compiling with GCC 13 (#8932) ## 16.0.139 stable 2023-03-16 @@ -2262,7 +2262,7 @@ * fix(developer): URL parameters should be UTF-8 (#7631) * fix(android/engine): Use IME package name if query permission denied (#7668) * fix(linux): Fix keyboard icon in system tray (#7678) -* chore(linux): Update debian changelog :cherries: (#7682) +* chore(linux): Update debian changelog (#7682) * chore(linux): Update Debian standards version (#7683) ## 16.0.100 beta 2022-11-10 @@ -2476,7 +2476,7 @@ ## 16.0.70 alpha 2022-09-26 * fix(android): Add language name when installing default lexical-model (#7347) -* chore(common): Add 15.0 stable entries to HISTORY.md :cherries: (#7350) +* chore(common): Add 15.0 stable entries to HISTORY.md (#7350) ## 16.0.69 alpha 2022-09-21 @@ -2666,7 +2666,7 @@ ## 16.0.37 alpha 2022-07-22 * fix(linux): Catch PermissionError exception (#6968) -* chore(linux): Update Debian changelog :cherries: (#6973) +* chore(linux): Update Debian changelog (#6973) * chore(linux): Add support for Ubuntu 22.10 "Kinetic Kudu" (#6975) * fix(developer): reduce timeouts if server shut down (#6943) * fix(linux): Implement refresh after keyboard installation (#6956) @@ -2859,12 +2859,12 @@ ## 15.0.269 stable 2022-08-29 * chore(linux): Update debian changelog (#7040) -* feat(linux): Replace deprecated distutils :cherries: (#7052) +* feat(linux): Replace deprecated distutils (#7052) * fix(developer): compiler emitting garbage for readonly groups (#7014) * chore: Change platform advocates per discussion (#7114) * fix(windows): remove saving and restoring context kbd options (#7107) * fix(windows): Add invalidate context action to non-updatable parse (#7108) -* fix(android/engine): :cherries: Lower the max height for landscape orientation (#7128) +* fix(android/engine): Lower the max height for landscape orientation (#7128) ## 15.0.268 stable 2022-08-04 @@ -2881,10 +2881,10 @@ * fix(ios): ignore CFBundleShortVersionString (#6935) * fix(web): context-only rule effects, set(&layer) from physical keystrokes ️ (#6949) * chore(web): remove invalid warning msg (#6951) -* fix(common): Fix `delete` :cherries: (#6966) -* fix(linux): Another attempt at fixing postinst script :cherries: (#6961) -* fix(linux): Fix uninstallation when using fcitx5 :cherries: (#6964) -* fix(linux): Catch PermissionError exception :cherries: (#6969) +* fix(common): Fix `delete` (#6966) +* fix(linux): Another attempt at fixing postinst script (#6961) +* fix(linux): Fix uninstallation when using fcitx5 (#6964) +* fix(linux): Catch PermissionError exception (#6969) * chore(linux): Update Debian changelog (#6972) * fix(developer): kmdecomp virtual character key output (#6945) * fix(developer): crash on exit when checking for updates (#6946) @@ -2896,7 +2896,7 @@ ## 15.0.266 stable 2022-07-08 -* fix(linux): Fix post-install script :cherries: (#6895) +* fix(linux): Fix post-install script (#6895) * fix(web): improve `console.error()` reporting (#6904) * fix(web): ncaps rules not matching on touch (#6913) @@ -3346,7 +3346,7 @@ ## 15.0.183 alpha 2022-01-21 -* chore(linux): Update changelogs for 14.0.284 :cherries: (#6132) +* chore(linux): Update changelogs for 14.0.284 (#6132) * chore(linux): Revert workaround for Python bug (#6133) ## 15.0.182 alpha 2022-01-21 @@ -3465,12 +3465,12 @@ ## 15.0.162 alpha 2021-12-05 -* chore(linux): Update changelogs for 14.0.283 :package: :cherries: (#6008) +* chore(linux): Update changelogs for 14.0.283 (#6008) ## 15.0.161 alpha 2021-12-04 -* chore(linux): Allow to specify debian revision :package: :cherries: (#5999) -* chore(linux): Remove lintian warning :package: :cherries: (#6000) +* chore(linux): Allow to specify debian revision (#5999) +* chore(linux): Remove lintian warning (#6000) ## 15.0.160 alpha 2021-12-03 @@ -3656,7 +3656,7 @@ * fix(android/engine): Remove unnecessary permissions from Manifest (#5752) * feat(developer): touch layout testing (#5723) * fix(web): popup positioning (#5742) -* chore(linux): Update changelog files for 14.0.282 :cherries: (#5794) +* chore(linux): Update changelog files for 14.0.282 (#5794) ## 15.0.124 alpha 2021-10-04 @@ -4472,8 +4472,8 @@ ## 14.0.285 stable 2022-01-20 -* fix(linux): Fix lintian errors :cherries: (#6107) -* fix(linux): Add workaround for Python bug :cherries: (#6125) +* fix(linux): Fix lintian errors (#6107) +* fix(linux): Add workaround for Python bug (#6125) * fix(web): Use regex to determine display layer and functional layers (#6123) ## 14.0.284 stable 2022-01-11 @@ -4484,12 +4484,12 @@ * fix(android/engine): Fix font paths (#5990) * chore(linux): Remove lintian warning (#5993) * chore(linux): Allow to specify debian revision (#5998) -* chore(linux): Update changelogs for 14.0.283 :package: (#6007) -* fix(linux): fix release version number for Sentry reporting :cherries: (#6053) +* chore(linux): Update changelogs for 14.0.283 (#6007) +* fix(linux): fix release version number for Sentry reporting (#6053) * chore(common): Check in crowdin strings for Spanish (Latin America) (#6060) -* fix(linux): fix release version number for Sentry reporting :cherries: (#6069) +* fix(linux): fix release version number for Sentry reporting (#6069) * chore(android/samples): Add -no-daemon flag to KMSample2 build script (#6083) -* fix(linux): Fix attribute error :cherries: (#6087) +* fix(linux): Fix attribute error (#6087) ## 14.0.283 stable 2021-11-17 @@ -4497,7 +4497,7 @@ * chore(linux): copy Keyman for Linux 15 reference to 14 (#5764) * fix(linux): Fix debian package script (#5772) * chore(common): Enhance cherry-pick labeling (#5773) -* fix(common): Fix cherry-pick labeling (:cherries:) (#5783) +* fix(common): Fix cherry-pick labeling () (#5783) * fix(linux): Don't crash displaying keyboard details (#5757) * chore(linux): Update changelog files for 14.0.282 (#5793) * fix(linux): Don't crash with non-keyboard package file (#5754) @@ -6016,7 +6016,7 @@ ## 14.0.81 alpha 2020-05-27 -* refactor(resources): convert gosh into npm package 🙃 (#3159) +* refactor(resources): convert gosh into npm package (#3159) * chore(common,web): use consistent TypeScript dep on all packages (#3158) * chore(common/resources): add `common/models` to build trigger definitions (#3144) * fix(common/resources): adds package-lock.json for gosh package (#3171) diff --git a/ios/tools/prepRelease.sh b/ios/tools/prepRelease.sh index c36ad97b45..0da59d2174 100755 --- a/ios/tools/prepRelease.sh +++ b/ios/tools/prepRelease.sh @@ -50,9 +50,8 @@ get_version_notes "ios" "${BUILD_NUMBER}" "$TIER" > $CHANGELOG_PATH echo "* Minor fixes and performance improvements" >> $CHANGELOG_PATH assertFileExists "${CHANGELOG_PATH}" -# Strip emoji to make Apple happy -emoji="\U1f300-\U1f5ff\U1f900-\U1f9ff\U1f600-\U1f64f\U1f680-\U1f6ff\U2600-\U26ff\U2700-\U27bf\U1f1e6-\U1f1ff\U1f191-\U1f251\U1f004\U1f0cf\U1f170-\U1f171\U1f17e-\U1f17f\U1f18e\U3030\U2b50\U2b55\U2934-\U2935\U2b05-\U2b07\U2b1b-\U2b1c\U3297\U3299\U303d\U00a9\U00ae\U2122\U23f3\U24c2\U23e9-\U23ef\U25b6\U23f8-\U23fa" -LC_ALL=UTF-8 sed -e "s/[$(printf $emoji)]//g" < "$CHANGELOG_PATH" > "$CHANGELOG_PATH.1" +# Strip emoji as App Store does not allow emoji in changelogs +node "$KEYMAN_ROOT/resources/tools/strip-emoji" < "$CHANGELOG_PATH" > "$CHANGELOG_PATH.1" mv -f "$CHANGELOG_PATH.1" "$CHANGELOG_PATH" assertFileExists "${CHANGELOG_PATH}" diff --git a/package-lock.json b/package-lock.json index 67ce85d1db..918bbca135 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,6 +7,7 @@ "name": "root", "workspaces": [ "resources/gosh", + "resources/tools/strip-emoji", "resources/build/version", "core/include/ldml", "developer/src/common/web/test-helpers", @@ -11672,6 +11673,10 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/stripemoji": { + "resolved": "resources/tools/strip-emoji", + "link": true + }, "node_modules/supports-color": { "version": "7.2.0", "license": "MIT", @@ -12702,6 +12707,7 @@ "dependencies": { "@actions/core": "^1.9.1", "@actions/github": "^2.1.0", + "emoji-regex": "^10.3.0", "typescript": "^4.9.5", "yargs": "^17.7.2" }, @@ -12739,6 +12745,11 @@ "node": ">=12" } }, + "resources/build/version/node_modules/emoji-regex": { + "version": "10.3.0", + "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-10.3.0.tgz", + "integrity": "sha512-QpLs9D9v9kArv4lfDEgg1X/gN5XLnf/A6l9cs8SPZLRZR3ZkY9+kwIQTxm+fsSej5UMYGE8fdoaZVIBlqG0XTw==" + }, "resources/build/version/node_modules/is-fullwidth-code-point": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-3.0.0.tgz", @@ -12760,6 +12771,11 @@ "node": ">=8" } }, + "resources/build/version/node_modules/string-width/node_modules/emoji-regex": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", + "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==" + }, "resources/build/version/node_modules/strip-ansi": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz", @@ -12803,6 +12819,19 @@ "gosh": "gosh.js" } }, + "resources/tools/strip-emoji": { + "name": "stripemoji", + "version": "1.0.0", + "license": "MIT", + "dependencies": { + "emoji-regex": "^10.3.0" + } + }, + "resources/tools/strip-emoji/node_modules/emoji-regex": { + "version": "10.3.0", + "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-10.3.0.tgz", + "integrity": "sha512-QpLs9D9v9kArv4lfDEgg1X/gN5XLnf/A6l9cs8SPZLRZR3ZkY9+kwIQTxm+fsSej5UMYGE8fdoaZVIBlqG0XTw==" + }, "web": { "name": "keyman", "license": "MIT", diff --git a/package.json b/package.json index aab36c3bc1..51fc2b99e5 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "scripts": {}, "workspaces": [ "resources/gosh", + "resources/tools/strip-emoji", "resources/build/version", "core/include/ldml", "developer/src/common/web/test-helpers", diff --git a/resources/build/version/package.json b/resources/build/version/package.json index 57264f02e4..5d55f9798f 100644 --- a/resources/build/version/package.json +++ b/resources/build/version/package.json @@ -4,6 +4,7 @@ "dependencies": { "@actions/core": "^1.9.1", "@actions/github": "^2.1.0", + "emoji-regex": "^10.3.0", "typescript": "^4.9.5", "yargs": "^17.7.2" }, diff --git a/resources/build/version/src/reportHistory.ts b/resources/build/version/src/reportHistory.ts index 2fae1ab628..e3c670265f 100644 --- a/resources/build/version/src/reportHistory.ts +++ b/resources/build/version/src/reportHistory.ts @@ -4,6 +4,7 @@ import { GitHub } from '@actions/github'; import { findLastHistoryPR, getAssociatedPR} from './graphql/queries.js'; import { spawnChild } from './util/spawnAwait.js'; +import emojiRegex from 'emoji-regex'; const getPullRequestInformation = async ( octokit: GitHub, base: string @@ -125,10 +126,10 @@ export const reportHistory = async ( } else { let git_tag = 'Next Version', git_tag_data = 'Next Version'; const re = /#(\d+)/; - const emojiRE = /[\u{1f300}-\u{1f5ff}\u{1f900}-\u{1f9ff}\u{1f600}-\u{1f64f}\u{1f680}-\u{1f6ff}\u{2600}-\u{26ff}\u{2700}-\u{27bf}\u{1f1e6}-\u{1f1ff}\u{1f191}-\u{1f251}\u{1f004}\u{1f0cf}\u{1f170}-\u{1f171}\u{1f17e}-\u{1f17f}\u{1f18e}\u{3030}\u{2b50}\u{2b55}\u{2934}-\u{2935}\u{2b05}-\u{2b07}\u{2b1b}-\u{2b1c}\u{3297}\u{3299}\u{303d}\u{00a9}\u{00ae}\u{2122}\u{23f3}\u{24c2}\u{23e9}-\u{23ef}\u{25b6}\u{23f8}-\u{23fa}]/gu; + const emojiRE = emojiRegex(); for(const commit of new_commits) { if(!useGitHubPRInfo) { - const git_pr_title = (await spawnChild('git', ['log', '--format=%b', '-n', '1', commit])).replace(emojiRE, ' ').trim(); + const git_pr_title = (await spawnChild('git', ['log', '--format=%b', '-n', '1', commit])).replaceAll(emojiRE, ' ').trim(); if(git_pr_title.match(/^auto\:/)) continue; const git_pr_data = (await spawnChild('git', ['log', '--format=%s', '-n', '1', commit])).trim(); const this_git_tag = (await spawnChild('git', ['tag', '--points-at', commit])).trim(); @@ -164,7 +165,7 @@ export const reportHistory = async ( if(pulls.find(p => p.number == pr.number) == undefined) { pr.tag_data = git_tag_data; pr.version = git_tag; - pr.title = pr.title.replace(emojiRE, ' ').trim(); + pr.title = pr.title.replaceAll(emojiRE, ' ').trim(); pulls.push(pr); } } diff --git a/resources/tools/strip-emoji/index.js b/resources/tools/strip-emoji/index.js new file mode 100644 index 0000000000..c255983208 --- /dev/null +++ b/resources/tools/strip-emoji/index.js @@ -0,0 +1,18 @@ + +const emojiRegex = require('emoji-regex'); + +process.stdin.setEncoding('utf-8'); + +// We will concatenate all strings and assume we are not processing a huge file, +// so we don't break in the middle of a UTF-8 sequence or split emoji sequences +// in half. + +let stream = ''; +process.stdin.on('readable', () => { + const data = process.stdin.read(); + if(data) { + stream += data; + } else { + process.stdout.write(stream.replaceAll(emojiRegex(), '')); + } +}); \ No newline at end of file diff --git a/resources/tools/strip-emoji/package.json b/resources/tools/strip-emoji/package.json new file mode 100644 index 0000000000..d28aebcc39 --- /dev/null +++ b/resources/tools/strip-emoji/package.json @@ -0,0 +1,14 @@ +{ + "name": "stripemoji", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "MIT", + "dependencies": { + "emoji-regex": "^10.3.0" + } +} From f4fe095f10237d03767a88f901b36a10532cdfe4 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Thu, 18 Apr 2024 11:58:13 +0700 Subject: [PATCH 18/18] change(web): adjusts multitap timings --- web/src/engine/osk/src/input/gestures/specsForLayout.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/engine/osk/src/input/gestures/specsForLayout.ts b/web/src/engine/osk/src/input/gestures/specsForLayout.ts index 9284265e3a..4731816a53 100644 --- a/web/src/engine/osk/src/input/gestures/specsForLayout.ts +++ b/web/src/engine/osk/src/input/gestures/specsForLayout.ts @@ -109,8 +109,8 @@ export const DEFAULT_GESTURE_PARAMS: GestureParams = { noiseTolerance: 10 }, multitap: { - waitLength: 500, - holdLength: 500 + waitLength: 300, + holdLength: 150 }, // Note: all actual runtime values are determined at runtime based upon row height. // See `VisualKeyboard.refreshLayout`, CTRL-F "Step 3".