From 0741b88dc2564ba148766ea7ccebb413eec27dd2 Mon Sep 17 00:00:00 2001
From: Marc Durdin <marc@durdin.net>
Date: Fri, 17 Jun 2022 05:17:05 +1000
Subject: [PATCH] fix(web): crash on custom modifier keys

Fixes #6788.
Fixes KEYMAN-WEB-22.
Fixes KEYMAN-DEVELOPER-BQ.
Fixes KEYMAN-ANDROID-160.

Fixes a crash introduced in #6473 which was trying to prevent modifier
keys from triggering "fat finger" alternate lookups, due to incomplete
nullish coalescing.

Related to this, from what I can see, a secondary side-effect of the fix
in #6473 was that some key events may have never had their ruleBehaviors
finalized if they matched the `isOnlyLayerSwitchKey` heuristic, leading
to potential issues with edge case 'deadkey+layer switch' keys or keys
that set store values, for example. So this fix also makes the
`isOnlyLayerSwitchKey` test more targeted.

Identified this when adding a Caps Lock layer to sil_euro_latin and
testing the layer switching. At time of fix, the error had been raised
in KeymanWeb, Keyman Developer and Keyman for Android, but not yet
Keyman for iPhone and iPad.
---
 .../input-processor/src/text/inputProcessor.ts   | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/common/web/input-processor/src/text/inputProcessor.ts b/common/web/input-processor/src/text/inputProcessor.ts
index 17bbbd2a10..d7d274bb42 100644
--- a/common/web/input-processor/src/text/inputProcessor.ts
+++ b/common/web/input-processor/src/text/inputProcessor.ts
@@ -141,27 +141,29 @@ namespace com.keyman.text {
 
       // If it's a key that we 'optimize out' of our fat-finger correction algorithm,
       // we MUST NOT trigger it for this keystroke.
-      let isOnlyLayerShift = text.Codes.isKnownOSKModifierKey(keyEvent.kName);
+      let isOnlyLayerSwitchKey = text.Codes.isKnownOSKModifierKey(keyEvent.kName);
 
       // Best-guess stopgap for possible custom modifier keys.
       // If a key (1) does not affect the context and (2) shifts the active layer,
       // we assume it's a modifier key.  (Touch keyboards may define custom modifier keys.)
       //
-      // Note:  this could cause an issue in the niche scenario where:
+      // Note:  this will mean we won't generate alternates in the niche scenario where:
       // 1.  Keypress does not alter the actual context
       // 2.  It DOES emit a deadkey with an earlier processing rule.
       // 3.  The FINAL processing rule does not match.
       // 4.  The key ALSO signals a layer shift.
       // If any of the four above conditions aren't met - no problem!
       // So it's a pretty niche scenario.
-      if((ruleBehavior.transcription?.transform as TextTransform).isNoOp() && keyEvent.kNextLayer) {
-        isOnlyLayerShift = true;
+      if((ruleBehavior?.transcription?.transform as TextTransform)?.isNoOp() && keyEvent.kNextLayer) {
+        isOnlyLayerSwitchKey = true;
       }
 
       const keepRuleBehavior = ruleBehavior != null;
       // Should we swallow any further processing of keystroke events for this keydown-keypress sequence?
-      if(keepRuleBehavior && !isOnlyLayerShift) {
-        let alternates = this.buildAlternates(ruleBehavior, keyEvent, preInputMock);
+      if(keepRuleBehavior) {
+        // alternates are our fat-finger alternate outputs. We don't build these for keys we detect as
+        // layer switch keys
+        let alternates = isOnlyLayerSwitchKey ? null : this.buildAlternates(ruleBehavior, keyEvent, preInputMock);
 
         // Now that we've done all the keystroke processing needed, ensure any extra effects triggered
         // by the actual keystroke occur.
@@ -173,7 +175,7 @@ namespace com.keyman.text {
         if(alternates && alternates.length > 0) {
           ruleBehavior.transcription.alternates = alternates;
         }
-      } else if(ruleBehavior == null) {
+      } else {
         // We need a dummy RuleBehavior for keys which have no output (e.g. Shift)
         ruleBehavior = new RuleBehavior();
         ruleBehavior.transcription = outputTarget.buildTranscriptionFrom(outputTarget, null, false);