fix(windows): add null pointer checks imsample keyboard#6187
Conversation
User Test ResultsTest specification and instructions ✅ SUITE_IMSAMPLE_KEYBOARD_INLINE_MENU:3 tests in 1 groups PASSED✅ SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW:2 tests in 1 groups PASSEDTest Artifacts |
|
We will test after all |
Hmm, given this was discovered in testing, shouldn't we be testing the fix? |
| Size=15 | ||
| gridx=30 | ||
| gridy=30 |
There was a problem hiding this comment.
Are these changes related to the fix?
There was a problem hiding this comment.
Yes. - this was one of the main causes for crashes. adding it though masked the crash so I removed it again and tried to fix all the crashes (null pointers or divide by zero errors) before putting it back. The main reason I didn't have the errors the tester had was because I had put this in directly on my local drive and had not built it as part of the keyboard package
Happy to. As I said in this description " I built in and attached to the PR the tester was verifying. I am just now checking in this branch." However, it was a quick fix, it still has many improvements to be made to this sample. For example the IM window display does not fit the grid of characters well after the base character is selected. For example type |
User TestingThis is the keyboard to be tested. imsample.zip I have copied the tests from #5936 and made a few modifications. SUITE_IMSAMPLE_KEYBOARD_INLINE_MENU:This keyboard uses the letters In this mode IM window is not shown rather the app text is updated with a menu when either
SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW:This keyboard uses the letters The first time you use the keyboard with an application the registry keys will be created and by default set to 1. If the IM window is not appearing you will need to verify the value of two registry keys. Note for the IM window you need to use the mouse pointer to select the options, you can't type the number.
|
SUITE_IMSAMPLE_KEYBOARD_INLINE_MENU
|
@MakaraSok @keymanapp-test-bot retest SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW all |
Retested and it shows that the IME doesn't seem to be active. Nothing pops up to select from. |
|
@rc-swag Tried to test SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW part and here is my observations: So, I got stuck on the problem. I have attached the Screenshot for reference. However, I tried to follow the steps as it mentioned in the TEST_IMSAMPLE_INPUT_IMW. When I typed some keys like t r y, nothing will show in the Notepad application. I need your help to test this issue. Or Did I missed out anything? |
|
@bharanidharanj You are supposed to add the two keys to the location in RegEditor.
@rc-swag I've tried this a few times and it seems like the IM is inactive. Pressing any of these |
@MakaraSok Ross and I tried to add the two keys in the above mentioned location in RegEditor. After adding the two keys, we noticed that the IM is still inactive. Typing any letter on the the Notepad does not show up anything. Anyhow, Ross said that he will look in to this issue and will update it. |
SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW:
|
Co-authored-by: Marc Durdin <marc@durdin.net>
397086e to
50f8c7b
Compare
This commit adds a function that sets the IM window registry keys needed for the IMSample keyboard. It also adds a null pointer check on the lpActiveKeyboard in the imx callback module.
|
@bharanidharanj and @MakaraSok @keymanapp-test-bot retest SUITE_IMSAMPLE_KEYBOARD_IM_WINDOW all |
|
Changes in this pull request will be available for download in Keyman version 15.0.207-beta |
1 similar comment
|
Changes in this pull request will be available for download in Keyman version 15.0.207-beta |
|
Changes in this pull request will be available for download in Keyman version 15.0.209-beta |
@rc-swag Okay, Thanks. |




tl;dr add null ptr checks to pointers before access.
The tester discovered crashed when loading the imsample keyboard after installing it. I went through and added checks throughout the code. I built in and attached to the PR the tester was verifying. I am just now checking in this branch.
Note: I made the coding style the current Keyman style. Normally on an old project like this, I would be happy to make the new code match the current code. If that is what you prefer please comment and I will change it.