Skip to content

chore(windows): remove WM_UNICHAR completely#8744

Merged
rc-swag merged 4 commits intochore/windows/8681/remove-addinsfrom
chore/windows/8677/remove-wm-unichar
May 15, 2023
Merged

chore(windows): remove WM_UNICHAR completely#8744
rc-swag merged 4 commits intochore/windows/8681/remove-addinsfrom
chore/windows/8677/remove-wm-unichar

Conversation

@rc-swag
Copy link
Copy Markdown
Contributor

@rc-swag rc-swag commented May 10, 2023

Fixes: #8677

For this PR we can just load two different keyman keyboards and test that characters are still being output

User Testing

TEST_TAMIL_OUTPUT

Install Tamil 99 keyboard

  • Open Notepad, select the Tamil Keyboard
  • Type in some characters and ensure they are output
  • Open MS Word if you have it otherwise, Firefox
  • Verify that typing produces the expected output

TEST_KHMER_ANGKOR

Install Khmer Angkor keyboard

  • Open Notepad, select the Khmer_Angkor Keyboard
  • Type in some characters and ensure they are output
  • Open MS Word if you have it otherwise, Firefox
  • Verify that typing produces the expected output

@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label May 10, 2023
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented May 10, 2023

User Test Results

Test specification and instructions

  • TEST_TAMIL_OUTPUT (PASSED): Tested with the attached PR build (Keyman 17.0.87-alpha-test-8744) in Windows 11 OS (VM) and here is my observation: 1. Installed Tamil 99 keyboard. 2. Able to use Tamil keyboard in the Notepad app as well as in the Firefox browser. Thanks, Ross. (notes)
  • TEST_KHMER_ANGKOR (PASSED): 1. Installed Khmer Angkor keybord. 2. Able to use the Khmer Angkor keyboard to type in khmer in the Notepad app as well as in the Firefox browser. Seems to be working fine. Thanks. (notes)

Test Artifacts

@rc-swag rc-swag added windows/ windows/engine/ and removed user-test-missing User tests have not yet been defined for the PR labels May 10, 2023
@rc-swag rc-swag added this to the A17S12 milestone May 10, 2023
@github-actions github-actions bot added the chore label May 10, 2023
@keymanapp-test-bot keymanapp-test-bot bot added user-test-missing User tests have not yet been defined for the PR has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels May 10, 2023
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very quick comments from our paired review

Comment on lines -221 to -230
(unsigned int) GetMessageExtraInfo());
else if(msg->message >= WM_KEYDOWN && msg->message <= WM_UNICHAR)
wsprintf(ds, "DebugMessage(%x, %s, wParam: '%c' (U+%04X), lParam: %X) [message flags: %x time: %d extra: %x]",
PtrToInt(msg->hwnd),
msgnames[msg->message-WM_KEYDOWN],
msg->wParam,
msg->wParam,
(unsigned int) msg->lParam,
wParam,
(int) msg->time,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave this as is, don't delete

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code hasn't yet been restored

@rc-swag rc-swag marked this pull request as ready for review May 11, 2023 04:49
@rc-swag rc-swag requested a review from ermshiperete as a code owner May 11, 2023 04:49
@mcdurdin mcdurdin changed the title chore(windows): remove WM_UNICHAR completley chore(windows): remove WM_UNICHAR completely May 11, 2023
Comment on lines -221 to -230
(unsigned int) GetMessageExtraInfo());
else if(msg->message >= WM_KEYDOWN && msg->message <= WM_UNICHAR)
wsprintf(ds, "DebugMessage(%x, %s, wParam: '%c' (U+%04X), lParam: %X) [message flags: %x time: %d extra: %x]",
PtrToInt(msg->hwnd),
msgnames[msg->message-WM_KEYDOWN],
msg->wParam,
msg->wParam,
(unsigned int) msg->lParam,
wParam,
(int) msg->time,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code hasn't yet been restored

@mcdurdin mcdurdin modified the milestones: A17S12, A17S13 May 14, 2023
@bharanidharanj
Copy link
Copy Markdown

Test Results

  • TEST_TAMIL_OUTPUT (PASSED): Tested with the attached PR build (Keyman 17.0.87-alpha-test-8744) in Windows 11 OS (VM) and here is my observation: 1. Installed Tamil 99 keyboard. 2. Able to use Tamil keyboard in the Notepad app as well as in the Firefox browser. Thanks, Ross.

..in NotePad

..in Firefox browser

  • TEST_KHMER_ANGKOR (PASSED): 1. Installed Khmer Angkor keybord. 2. Able to use the Khmer Angkor keyboard to type in khmer in the Notepad app as well as in the Firefox browser. Seems to be working fine. Thanks.

..in Notepad

.. Firefox browser

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label May 15, 2023
@rc-swag rc-swag merged commit f8a351a into chore/windows/8681/remove-addins May 15, 2023
@rc-swag rc-swag deleted the chore/windows/8677/remove-wm-unichar branch May 15, 2023 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants