fix(linux): Improve setting context

[ermshiperete]

This change always gets the context from the surrounding text if reset_context gets called. This helps when the user puts the IP after a certain character. Previously we basically lost the context, with this change we restore the context so that it's like the user just typed the previous characters.

It might be easiest to review the individual commits in this PR.

[keymanapp-test-bot[bot]]

User Test Results

Test specification and instructions

• ✅ GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11

  6 tests PASSED

  • ✅ TEST_IPA_SAMEAPP (PASSED): Installed Keyman 16.0.83-alpha build in Bionic Linux OS (VM) . Tested this as per the instructions and verified that after pressing > key, the letter n was changed into ŋ. (notes)
  • ✅ TEST_IPA_TWOAPPS (PASSED): Tested this as per the instructions in gedit and gnome terminal and after the switching I was able to change the n character into ŋ by pressing the > key. (notes)
  • ✅ TEST_DEADKEY_SAMEAPP (PASSED): Tested this as per the instructions and verified the deadkeys are not working after position changes. (notes)
  • ✅ TEST_DEADKEY_TWOAPPS (PASSED): ): the result is correct, my instructions were wrong
  • ✅ TEST_GEDIT_k019 (PASSED): Tested this as per the instructions and verified the expected output on the text editor. (notes)
  • ✅ TEST_TERM_k019 (PASSED): Tested this as per the instructions and verified the expected output appears on the gnome-terminal. (notes)

• ✅ GROUP_JAMMY_X11: Ubuntu 22.04 Jammy with Gnome Shell and X11

  6 tests PASSED

  • ✅ TEST_IPA_SAMEAPP (PASSED): Installed Keyman 16.0.83-alpha build in Jammy Linux OS (VM) . Tested this as per the instructions and verified that after pressing > key, the letter n was changed into ŋ. (notes)
  • ✅ TEST_IPA_TWOAPPS (PASSED): Tested this as per the instructions in gedit and gnome terminal and after the switching I was able to change the n character into ŋ by pressing the > key. (notes)
  • ✅ TEST_DEADKEY_SAMEAPP (PASSED): Tested this as per the instructions and verified the deadkeys are not working after position changes. (notes)
  • ✅ TEST_DEADKEY_TWOAPPS (PASSED): ): the result is correct, my instructions were wrong
  • ✅ TEST_GEDIT_k019 (PASSED): Tested this as per the instructions and verified the expected output on the text editor. (notes)
  • ✅ TEST_TERM_k019 (PASSED): Retested this as per Eberhard's suggestion (ie., running the dpkg -l ibus command on the terminal) and I was able to see the expected output. (notes)

Test Artifacts

• Linux
  • Keyman for Linux

[ermshiperete]

@mcdurdin This seems like a good idea because it allows you to pick up where you left of before you switched to another app, but I'm not entirely sure it doesn't have side effects that I'm currently not seeing.

[mcdurdin]

@mcdurdin This seems like a good idea because it allows you to pick up where you left of before you switched to another app, but I'm not entirely sure it doesn't have side effects that I'm currently not seeing.

From my understanding, this is looking at picking up context when insertion point changes, where the app has surrounding-text support. We should certainly be doing this. However, I am not sure that this is quite right yet. A few thoughts, if the insertion point moves (apart from when we are inserting text), then:

1. We should always abandon any cached context, whether or not the app supports surrounding-text.
2. We don't need to compare the surrounding-text length to the cached context. Just throw away the cached context in this situation.
3. If surrounding-text is available, definitely read it in.
4. We should ensure we clear any deadkey state as well.

This insertion point movement applies to focus changes within the current app, mouse clicks, text selection, executing commands, hotkeys, etc. If the user switches apps, we should reset state in the same way: it is not generally helpful to track deadkey state as the user won't remember which deadkey they have pressed in the app anyway.

[ermshiperete]

Thanks for your thoughts, @mcdurdin! The context now gets always reset in reset_context which should take care of the points you mentioned, and also simplifies the code :smile:

[ermshiperete]

This change still has a problem when the keyboard outputs a character + deadkey. In response to outputting the character we get a ibus_keyman_engine_set_surrounding_text call in which we call reset_context which removes the deadkey marker. Even if we wouldn't call reset_context and directly call km_kbp_context_set we would lose the deadkey. Or should we make ibus_keyman_engine_set_surrounding_text a no-op?

Not sure how to deal with this. Any suggestions @mcdurdin or @rc-swag ?

[mcdurdin]

On Windows, we compare the new text context from the app to the textual part of the cached context. If it is different, then we reset the cached context and deadkeys. (In theory, we could have the same text in two places, but it's not particularly harmful in that rare situation anyway for the deadkey to 'transport' across text contexts.)

[rc-swag]

Even if we wouldn't call reset_context and directly call km_kbp_context_set we would lose the deadkey

In addition to what Marc has said, we have a platform side helper function ContextItemsFromAppContext which takes the windows platform context with deadkeys, and converts to the Keyman Engine API km_kbp_context_item list including the deadkey markers. Then we use km_kbp_context_set passing in this context with the deadkey markers [ See Process_Event_Core line 102 -104 of kmprocess.cpp].

The API calls km_kbp_context_items_from_utf** and km_kbp_context_items_to_utf strip the markers which is a gotcha to be aware off.

[bharanidharanj]

GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11

• TEST_IPA_SAMEAPP (PASSED): Installed Keyman 16.0.83-alpha build in Bionic Linux OS (VM) . Tested this as per the instructions and verified that after pressing > key, the letter n was changed into ŋ.

• TEST_IPA_TWOAPPS (PASSED): Tested this as per the instructions in gedit and gnome terminal and after the switching I was able to change the n character into ŋ by pressing the > key.

• TEST_DEADKEY_SAMEAPP (PASSED): Tested this as per the instructions and verified the deadkeys are not working after position changes.

• TEST_DEADKEY_TWOAPPS (FAILED): Tested this as per the instructions and noticed thta the letter á did not change in the gnome terminal. seems to be an issue.

• TEST_GEDIT_k019 (PASSED): Tested this as per the instructions and verified the expected output on the text editor.

• TEST_TERM_k019 (PASSED): Tested this as per the instructions and verified the expected output appears on the gnome-terminal.

[bharanidharanj]

GROUP_JAMMY_X11: Ubuntu 22.04 Jammy with Gnome Shell and X11

• TEST_IPA_SAMEAPP (PASSED): Installed Keyman 16.0.83-alpha build in Jammy Linux OS (VM) . Tested this as per the instructions and verified that after pressing > key, the letter n was changed into ŋ.

  

• TEST_IPA_TWOAPPS (PASSED): Tested this as per the instructions in gedit and gnome terminal and after the switching I was able to change the n character into ŋ by pressing the > key.

  

• TEST_DEADKEY_SAMEAPP (PASSED): Tested this as per the instructions and verified the deadkeys are not working after position changes.

  

• TEST_DEADKEY_TWOAPPS (FAILED): Tested this as per the instructions and noticed that the letter á did not change in the gnome terminal. seems to be an issue.

  

• TEST_GEDIT_k019 (PASSED): Tested this as per the instructions and verified the expected output on the text editor.

  

• TEST_TERM_k019 (FAILED): Tested this as per the instructions and noticed that it is showing wrong output.

  

[ermshiperete]

GROUP_BIONIC TEST_DEADKEY_TWOAPPS(PASSED): the result is correct, my instructions were wrong

GROUP_JAMMY_X11 TEST_DEADKEY_TWOAPPS(PASSED): the result is correct, my instructions were wrong

[ermshiperete]

@bharanidharanj Please make sure you use the new ibus version (check version with dpkg -l ibus; should be version *sil2.1*) and reboot after installation of ibus. Then I think the remaining failing test will work.

@keymanapp-test-bot retest GROUP_JAMMY_X11 TEST_TERM_k019

[bharanidharanj]

@bharanidharanj Please make sure you use the new ibus version (check version with dpkg -l ibus; should be version *sil2.1*) and reboot after installation of ibus. Then I think the remaining failing test will work.

@keymanapp-test-bot retest GROUP_JAMMY_X11 TEST_TERM_k019

@ermshiperete Okay, Thanks.

[bharanidharanj]

GROUP_JAMMY_X11: Ubuntu 22.04 Jammy with Gnome Shell and X11

• TEST_TERM_k019 (PASSED): Retested this as per Eberhard's suggestion (ie., running the dpkg -l ibus command on the terminal) and I was able to see the expected output.

  

[keyman-server]

Changes in this pull request will be available for download in Keyman version 16.0.89-alpha