fix(core): fix keydown/up handling for LDML keyboards #️⃣

core/src/km_core_processevent_api.cpp

@@ -50,6 +50,9 @@ km_core_process_event(km_core_state *state,
   if(state == nullptr) {
     return KM_CORE_STATUS_INVALID_ARGUMENT;
   }
+  if (vk == KM_CORE_VKEY_BKSP && is_key_down) {
+    state->set_backspace_handled_internally(false);
+  }
   km_core_status status = state->processor().process_event(state, vk, modifier_state, is_key_down, event_flags);
 
   if (state_should_invalidate_context(state, vk, modifier_state, is_key_down, event_flags)) {
@@ -78,6 +81,10 @@ km_core_process_event(km_core_state *state,
 
   state->apply_actions_and_merge_app_context();
 
+  if (vk == KM_CORE_VKEY_BKSP) {
+    state->set_backspace_handled_internally(!state->action_struct().emit_keystroke);
+  }
+
   return status;
 }
 

core/src/ldml/ldml_processor.cpp

@@ -179,21 +179,20 @@ ldml_processor::process_event(
   ldml_state.clear();
 
   try {
-    if (!is_key_down) {
-      process_key_up(ldml_state);
-    } else {
-      switch (vk) {
-      // Currently, only one VK gets spoecial treatment.
-      // Special handling for backspace VK
-      case KM_CORE_VKEY_BKSP:
-        process_backspace(ldml_state);
-        break;
-      default:
-        // all other VKs
+    switch (vk) {
+    // Currently, only one VK gets special treatment.
+    // Special handling for backspace VK
+    case KM_CORE_VKEY_BKSP:
+      process_backspace(ldml_state);
+      break;
+    default:
+      // all other VKs
+      if (is_key_down) {
         process_key_down(ldml_state);
-      } // end of switch
-    } // end of normal processing
-
+      } else {
+        process_key_up(ldml_state);
+      }
+    }  // end of switch
     // all key-up and key-down events end up here.
     // commit the ldml state into the core state
     ldml_state.commit();
@@ -210,11 +209,31 @@ void
 ldml_processor::process_key_up(ldml_event_state &ldml_state)
     const {
   // TODO-LDML: Implement caps lock handling
-  ldml_state.clear();
+
+  // Look up the key
+  bool found = false;
+  const std::u16string key_str = keys.lookup(ldml_state.get_vk(), ldml_state.get_modifier_state(), found);
+
+  if (!found) {
+    ldml_state.emit_passthrough_keystroke();
+  }
 }
 
 void
 ldml_processor::process_backspace(ldml_event_state &ldml_state) const {
+  if (ldml_state.get_modifier_state() & K_MODIFIERFLAG) {
+    // we never process modifier+bksp
+    ldml_state.emit_passthrough_keystroke();
+    return;
+  }
+
+  if (!ldml_state.is_key_down()) {
+    if (!ldml_state.get_state()->backspace_handled_internally()) {
+      ldml_state.emit_passthrough_keystroke();
+    }
+    return;
+  }
+
   if (!!bksp_transforms) {
     // process with an empty string via the bksp transforms
     auto matchedContext = process_output(ldml_state, std::u32string(), bksp_transforms.get());
@@ -243,7 +262,7 @@ void ldml_event_state::emit_backspace() {
     // else loop again
     assert(end.type != KM_CORE_CT_END);  // inappropriate here.
     state->context().pop_back();
- }
+  }
   /*
     We couldn't find a character at end of context (context is empty),
     so we'll pass the backspace keystroke on to the app to process; the
@@ -532,10 +551,10 @@ ldml_event_state::ldml_event_state(
     uint8_t i,
     uint16_t e) {
   this->state          = s;
-  this->vk             = v;
-  this->modifier_state = m;
-  this->is_key_down    = i;
-  this->event_flags    = e;
+  this->_vk             = v;
+  this->_modifier_state = m;
+  this->_is_key_down    = i;
+  this->_event_flags    = e;
 
   actions.persist_options = new km_core_option_item[1];
   actions.persist_options[0] = NULL_OPTIONS[0];

core/src/ldml/ldml_processor.hpp

@@ -176,12 +176,15 @@ class ldml_event_state {
     * @return the number of context items consumed
     */
    size_t context_to_string(std::u32string &str, bool include_markers = true);
+   km_core_state* get_state() const { return state; }
+
+   uint8_t is_key_down() const { return _is_key_down; }
 
  private:
-   km_core_virtual_key vk;
-   uint16_t modifier_state;
-   uint8_t is_key_down;
-   uint16_t event_flags;
+   km_core_virtual_key _vk;
+   uint16_t _modifier_state;
+   uint8_t _is_key_down;
+   uint16_t _event_flags;
    km_core_state *state;
 
    // our in-flight action struct.
@@ -194,12 +197,12 @@ class ldml_event_state {
 // implementation
 km_core_virtual_key
 ldml_event_state::get_vk() const {
-  return vk;
+  return _vk;
 }
 
 uint16_t
 ldml_event_state::get_modifier_state() const {
-  return modifier_state;
+  return _modifier_state;
 }
 
 } // namespace core

core/src/state.cpp

@@ -52,6 +52,7 @@ state::state(km::core::abstract_processor & ap, km_core_option_item const *env)
                      env->key,
                      env->value);
   }
+  _backspace_handled_internally = false;
   _imx_callback = nullptr;
   _imx_object = nullptr;
   memset(const_cast<km_core_actions*>(&_action_struct), 0, sizeof(km_core_actions));
@@ -117,4 +118,4 @@ void state::apply_actions_and_merge_app_context() {
   }
 
   this->_action_struct.deleted_context = km::core::get_deleted_context(app_context_for_deletion, this->_action_struct.code_points_to_delete);
-}
+}

core/src/state.hpp

@@ -130,6 +130,7 @@ class state
     core::debug_items          _debug_items;
     km_core_keyboard_imx_platform _imx_callback;
     void *_imx_object;
+    bool _backspace_handled_internally;
 
 public:
     state(core::abstract_processor & kb, km_core_option_item const *env);
@@ -174,6 +175,12 @@ class state
       km_core_actions const &actions
     );
     void apply_actions_and_merge_app_context();
+
+    // This is used to track whether the backspace key was handled internally
+    // during the keydown event. This is needed so that we can return the same
+    // value from the keyup event. Only used when processing KM_CORE_VKEY_BKSP.
+    void set_backspace_handled_internally(bool handled) { _backspace_handled_internally = handled; }
+    bool backspace_handled_internally() const { return _backspace_handled_internally; }
   };
 } // namespace core
 } // namespace km

core/tests/unit/km_core_keyboard_api.tests.cpp

@@ -18,7 +18,7 @@ class KmCoreKeyboardApiTests : public testing::Test {
       km_core_keyboard_dispose(this->keyboard);
       this->keyboard = nullptr;
     }
-    }
+  }
 };
 
 TEST_F(KmCoreKeyboardApiTests, LoadFromBlob) {