From 2d06c65412e7ab376b8aca2e5f839d98dd6101ac Mon Sep 17 00:00:00 2001
From: jahorton <jahorton@sil.org>
Date: Wed, 16 Jun 2021 16:10:49 +0700
Subject: [PATCH] refactor(web): simplifies embedded executePopupKey

---
 web/source/kmwembedded.ts        | 45 +++++---------------------------
 web/source/osk/visualKeyboard.ts | 11 +++++---
 2 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/web/source/kmwembedded.ts b/web/source/kmwembedded.ts
index a6ad1cde2f..1996a17e48 100644
--- a/web/source/kmwembedded.ts
+++ b/web/source/kmwembedded.ts
@@ -360,8 +360,8 @@ namespace com.keyman.text {
       
       keymanweb.domManager.initActiveElement(Lelem);
 
-      var nextLayer: string;
       var delegator: com.keyman.osk.embedded.SubkeyDelegator = null;
+      let selectedKey: com.keyman.osk.OSKKeySpec = null;
       // This should be set if we're within this method... but it's best to guard against nulls here, just in case.
       if(osk.vkbd.subkeyDelegator) {
         delegator = osk.vkbd.subkeyDelegator as com.keyman.osk.embedded.SubkeyDelegator;
@@ -371,15 +371,14 @@ namespace com.keyman.text {
         var found = false;
 
         if(baseKey.coreID == keyName) {
-          nextLayer = baseKey.nextlayer;
+          selectedKey = baseKey;
           found = true;
         } else {
           // Search for the specified subkey so we can retrieve its useful properties.
           // It should be within the popupBaseKey's subkey list.
           for(let subKey of baseKey.sk) {
             if(subKey.coreID == keyName) {
-              // ... to consider:  why are we not just taking the keyspec wholesale right here?
-              nextLayer = subKey.nextlayer;
+              selectedKey = subKey;
               found = true;
               break;
             }
@@ -395,42 +394,12 @@ namespace com.keyman.text {
         console.warn("No base key exists for the subkey being executed: '" + origArg + "'");
       }
 
-      let Codes = com.keyman.text.Codes;
-      
-      // Check the virtual key 
-      let Lkc: com.keyman.text.KeyEvent = {
-        Lmodifiers: keyShiftState,
-        Lstates: 0,
-        Lcode: Codes.keyCodes[keyName],
-        LisVirtualKey: true,
-        kName: keyName,
-        kNextLayer: nextLayer,
-        vkCode: null, // was originally undefined
-        isSynthetic: true,
-        device: keymanweb.util.device.coreSpec
-      };
-
-      // Process modifier key action
-      if(core.keyboardProcessor.selectLayer(Lkc, true)) { // ignores key's 'nextLayer' property for this check
-        return true;      
+      let Lkc: com.keyman.text.KeyEvent = null;
+      if(selectedKey) {
+        Lkc = osk.vkbd.keyEventFromSpec(selectedKey as com.keyman.keyboards.ActiveKey, null);
+        Lkc.vkCode=Lkc.Lcode;
       }
 
-      // While we can't source the base KeyEvent properties for embedded subkeys the same way as native,
-      // we can handle many other pre-processing steps the same way with this common method.
-      core.keyboardProcessor.setSyntheticEventDefaults(Lkc);
-
-      //if(!Lkc.Lcode) return false;  // Value is now zero if not known (Build 347)
-      //Build 353: revert to prior test to try to fix lack of KMEI output, May 1, 2014      
-      if(isNaN(Lkc.Lcode) || !Lkc.Lcode) { 
-        // Addresses modifier SHIFT keys.
-        if(nextLayer) {
-          core.keyboardProcessor.selectLayer(Lkc);
-        }
-        return false;
-      }
-
-      Lkc.vkCode=Lkc.Lcode;
-
       if(delegator) {
         delegator.resolve(Lkc);
       }
diff --git a/web/source/osk/visualKeyboard.ts b/web/source/osk/visualKeyboard.ts
index fbd09d3fce..5e6c407e81 100644
--- a/web/source/osk/visualKeyboard.ts
+++ b/web/source/osk/visualKeyboard.ts
@@ -960,8 +960,6 @@ namespace com.keyman.osk {
       // Turn off key highlighting (or preview)
       this.highlightKey(e,false);
 
-      let core = com.keyman.singleton.core; // only singleton-based ref currently needed here.
-
       // Future note:  we need to refactor osk.OSKKeySpec to instead be a 'tag field' for
       // keyboards.ActiveKey.  (Prob with generics, allowing the Web-only parts to
       // be fully specified within the tag.)  
@@ -973,7 +971,13 @@ namespace com.keyman.osk {
         console.error("OSK key with ID '" + e.id + "', keyID '" + e.keyId + "' missing needed specification");
         return null;
       }
+      
+      // Return the event object.
+      return this.keyEventFromSpec(keySpec, touch);
+    }
 
+    keyEventFromSpec(keySpec: keyboards.ActiveKey, touch?: Touch) {
+      let core = com.keyman.singleton.core; // only singleton-based ref currently needed here.
 
       // Start:  mirrors _GetKeyEventProperties
 
@@ -994,8 +998,7 @@ namespace com.keyman.osk {
         Lkc.source = touch;
         Lkc.keyDistribution = this.getTouchProbabilities(touch);;
       }
-      
-      // Return the event object.
+
       return Lkc;
     }