Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(windows): LDML keyboards do not appear to work correctly with Alt keys #12298

Open
mcdurdin opened this issue Aug 27, 2024 · 5 comments · May be fixed by #12644
Open

bug(windows): LDML keyboards do not appear to work correctly with Alt keys #12298

mcdurdin opened this issue Aug 27, 2024 · 5 comments · May be fixed by #12644
Assignees
Milestone

Comments

@mcdurdin
Copy link
Member

  • TEST_WINDOWS (Failed):
    Window 10 OS:
  1. I installed the Keyman 18.0.97-alpha version and then started the configuration.
  2. On Keyman developer: Navigate to the "Packaging" tab and then click the *.kmx link button to install the keyboard in Windows 10.
  3. Select the LDML keyboard that was installed through "Keyman Developer"
  4. Open the notepad. Press the below keys and observe the below options.
  5. q emits "D" and appears. It seems wrong.
  6. shift+q emits "B" appeared. It's correct
  7. ctrl+q emits "C" appeared. It's correct
  8. alt+q emits 🅓 no results
  9. alt+w emits 🅔 no results
  10. The below key combination does not give any outputs: It correct
    alt+e, ctrl+e, shift+e, ctrl+shift+e, ctrl+alt+e, e
    https://github.com/user-attachments/assets/7d1f1153-80d0-438d-874e-de32ffd619a8

Originally posted by @dinakaranr in #12281 (comment)

@rc-swag
Copy link
Contributor

rc-swag commented Nov 4, 2024

It is failing in Windows because the LDML keyboard processor has not been fully implemented ldml_processor::get_key_list() it just returns an empty list.

In Windows the Keyman engine needs to register the interest in the preserved keys otherwise it will not be seen in the hook for the Keyman keyboard processor.

Implementation detail:
Of interest with this example also is that the ALT key is in a keyboard rule via the the LDML rule other for modifiers. The get_key_list() will obviously have to do the logical expansion of the rule.

For more information
The Call stack will be
Keyman32Interface::GetKeyboardPreservedKeys
MapKeyboard

km_core_status err_status = km_core_keyboard_get_key_list(pKeyboard, &kb_key_list);

@srl295 Could you implement ldml_processor::get_key_list() ?
Some of the necessary processing may already be being done in the ldml_processor when passing the keyboard rules.

@srl295
Copy link
Member

srl295 commented Nov 5, 2024

I see three of these that don't have any 'interesting' implementation. i'll check over the docs.

    km_core_context_item * get_intermediate_context() override;
    km_core_keyboard_key  * get_key_list() const override;
    km_core_keyboard_imx  * get_imx_list() const override;

@mcdurdin
Copy link
Member Author

mcdurdin commented Nov 6, 2024

get_intermediate_context and get_imx_list are only really relevant for IMX, which LDML keyboards do not support.

srl295 added a commit that referenced this issue Nov 7, 2024
- move some test utils to statics
- add a new test action type and `@@key-list` keyword
- print warning on unhanded @-commands

Fixes: #12298
srl295 added a commit that referenced this issue Nov 7, 2024
- ldml::vkeys class updated to keep a set<> of keys
- fix the test case to not leak!

Fixes: #12298
@srl295 srl295 linked a pull request Nov 7, 2024 that will close this issue
@darcywong00 darcywong00 modified the milestones: A18S14, A18S15 Nov 9, 2024
@darcywong00 darcywong00 removed this from the A18S15 milestone Nov 24, 2024
@darcywong00 darcywong00 added this to the A18S16 milestone Nov 24, 2024
@rc-swag rc-swag moved this to In Progress in Keyman Nov 25, 2024
srl295 added a commit that referenced this issue Nov 25, 2024
- move kmxplus processing into the base LdmlTestSource class
- add a function to traverse the layer list looking for keys to add
- The @@keyList keyword only has one example from each modifier set

Fixes: #12298
srl295 added a commit that referenced this issue Nov 26, 2024
- just compare the key list to the key2.kmap table
- check the keylist for all LdmlTestSource instances - no syntax needed

Fixes: #12298
@srl295
Copy link
Member

srl295 commented Nov 26, 2024

Trying to repro the failing case.
Windows 11.
Keyman 18.0.116-alpha (will update and try again).
Notepad.

  • q emitted D briefly and then went to the 🅐.
  • alt-q and alt-w gave no output, but attempted to open the menu hotkeys.

@rc-swag
Copy link
Contributor

rc-swag commented Nov 27, 2024

  • alt-q and alt-w gave no output, but attempted to open the menu hotkeys.

This is reproducing the error. As the alt key has not been added a preserved key, see description above. Therefore the ALT key is going through to the application where is is treated and the menu key.

  • q emitted D briefly and then went to the 🅐.

Testing this again I can only reproduce it by having caps lock on - in which case D is correct. I am wondering if this was the case when Dina was testing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

5 participants
@srl295 @mcdurdin @darcywong00 @rc-swag and others