-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
fix(core): reset on frame keys 🙀 #11172
Conversation
- reverts 04a6e16 for k_102_keytest.xml
- update ldml test with an exception for k_102_keytest
User Test ResultsTest specification and instructions 🟩 SUITE_KMX_PROCESSOR_NON_COMPLIANT:
🟩 SUITE_KMX_PROCESSOR_COMPLIANT:
🟩 SUITE_LDML_PROCESSOR_NON_COMPLIANT:
🟩 SUITE_LDML_PROCESSOR_COMPLIANT:
Test Artifacts
|
This should be fixing that issue, right? in which case "fixes ..." rather than "for ..." (and don't forget to link in the Development right hand panel in GH)
I think we can usefully have user tests for this to verify that outcome is as we expect on each platform. Need to spec with non-compliant apps, a test for pressing specific frame key and another test for non-frame keys, with a keyboard rule that depends on preceding context. With sil_euro_latin, type ' ... frame key | non frame key ... e, and for non-frame key it should combine, with frame key it shouldn't. We could use e.g. Insert as a non-frame key, and e.g. Escape as a frame key, if using an app such as Notepad where those keys won't have any other effect. In fact may need to have tests with compliant apps as well, to verify markers are dropped when frame keys are pressed. Needs thinking through a bit but I think this is critical given this is fundamental to how Keyman works. |
I usually update that once it actually fixes.
Good point. I have never done a user test so far. So I'm just used to skip.
Makes sense |
@keymanapp-test-bot retest SUITE_KMX_PROCESSOR_NON_COMPLIANT GROUP_WIN_10 TEST_SCROLLLOCK_KEY_NO_RESET GROUP_WIN_10 TEST_SCROLLLOCK_KEY_NO_RESET SUITE_LDML_PROCESSOR_NON_COMPLIANT GROUP_WIN_10 TEST_FRAME_SCROLL_LOCK_NO_RESET SUITE_LDML_PROCESSOR_COMPLIANT GROUP_WIN_10 TEST_FRAME_KEY_RESET_NO_MARKERS TEST_FRAME_SCROLL_LOCK_NO_RESET |
@keymanapp-test-bot retest SUITE_LDML_PROCESSOR_NON_COMPLIANT GROUP_WIN_11 TEST_FRAME_KEY_RESET_NOMARKERS |
@bharanidharanj I forgot this one which was still on the wrong editor. Otherwise I think the only issue is the scroll lock key. PR broken due to only partly accepting review comments - sorry, fixed. |
@rc-swag same, if you get it sorted out please go ahead and merge |
@keymanapp-test-bot retest SUITE_KMX_PROCESSOR_NON_COMPLIANT GROUP_WIN_11 TEST_FRAME_KEY_RESET_NOMARKERS |
@keymanapp-test-bot retest SUITE_KMX_PROCESSOR_NON_COMPLIANT GROUP_LINUX TEST_CONTROL_1 SUITE_LDML_PROCESSOR_NON_COMPLIANT GROUP_WIN_11 TEST_FRAME_KEY_RESET_NO_MARKERS |
@keymanapp-test-bot retest SUITE_KMX_PROCESSOR_NON_COMPLIANT GROUP_WIN_11 TEST_SCROLLLOCK_KEY_NO_RESET SUITE_KMX_PROCESSOR_COMPLIANT GROUP_WIN_11 TEST_SCROLLLOCK_KEY_NO_RESET SUITE_LDML_PROCESSOR_NON_COMPLIANT GROUP_WIN_11 TEST_FRAME_SCROLL_LOCK_NO_RESET |
@keymanapp-test-bot retest SUITE_LDML_PROCESSOR_COMPLIANT GROUP_WIN_10 TEST_FRAME_SCROLL_LOCK_NO_RESET SUITE_LDML_PROCESSOR_COMPLIANT GROUP_WIN_11 TEST_FRAME_SCROLL_LOCK_NO_RESET |
Test ResultsSUITE_KMX_PROCESSOR_NON_COMPLIANT:GROUP_WIN_10: TextEditor
GROUP_WIN_11: TextEditor
SUITE_KMX_PROCESSOR_COMPLIANT:GROUP_WIN_11: Wordpad
SUITE_LDML_PROCESSOR_NON_COMPLIANT:GROUP_WIN_10: TextEditor
GROUP_WIN_11: TextEditor
GROUP_MAC: Chrome Browser
SUITE_LDML_PROCESSOR_COMPLIANT:GROUP_WIN_10: WordPad
GROUP_WIN_11: WordPad
|
Test ResultsSUITE_KMX_PROCESSOR_NON_COMPLIANT:GROUP_LINUX: Chrome Brower (https://www.editpad.org/)
|
Changes in this pull request will be available for download in Keyman version 17.0.315-beta |
Looks like we forgot to test this in the Keyman Developer debugger -- #11486! |
Fixes #10955
Fixes #10227
User Testing
Using Windows 10 or Windows 11.
Install the keyman build from this PR.
Use the Text Editor linked in this PR under test artifacts. or if on Windows 10 you can use Notepad.
SUITE_KMX_PROCESSOR_NON_COMPLIANT:
For these tests make sure the application used on the operating system is a non-compliant. A app that is not able to give the context to Keyman.
GROUP_WIN_10: TextEditor
GROUP_WIN_11: TextEditor
GROUP_MAC: Chrome Browser
GROUP_LINUX: Chrome Brower (https://www.editpad.org/)
Install SIL Euro Latin
TEST_CONTROL_1
Expected result:
è
TEST_FRAME_KEY_RESET_MARKERS
This tests the markers are lost
Expected result:
`e
TEST_FRAME_KEY_RESET_NOMARKERS
Expected result:
1//2
TEST_SCROLLLOCK_KEY_NO_RESET
Do not reset for scroll lock
Expected Result:
è
SUITE_KMX_PROCESSOR_COMPLIANT:
For these tests make sure the application used on the operating system is compliant. That is Keyman can request the context from the application.
GROUP_WIN_10: WordPad
GROUP_WIN_11: Wordpad
GROUP_MAC: TextEdit
GROUP_LINUX: gedit
Install SIL Euro Latin
TEST_CONTROL_1
Expected result:
è
TEST_FRAME_KEY_RESET_MARKERS
This tests the markers are lost
Expected result:
`e
TEST_FRAME_KEY_RESET_NOMARKERS
Expected result:
½
TEST_SCROLLLOCK_KEY_NO_RESET
Do not reset for scroll lock
Expected Result:
è
SUITE_LDML_PROCESSOR_NON_COMPLIANT:
LDML keyboard
GROUP_WIN_10: TextEditor
GROUP_WIN_11: TextEditor
GROUP_MAC: Chrome Browser
GROUP_LINUX: Chrome Brower
Install contextreset.zip ldml keyboard
TEST_CONTROL_1
Expected result:
ê
This was previously failing
TEST_FRAME_KEY_RESET_MARKERS
Expected result:
e
TEST_FRAME_KEY_RESET_NO_MARKERS
Expected result:
a*
TEST_FRAME_SCROLL_LOCK_NO_RESET
Expected result:
ê
TEST_MODIFIER_TAP_NO_RESET
Do not reset for modifier tap
Expected result:
ê
SUITE_LDML_PROCESSOR_COMPLIANT:
LDML keyboard
GROUP_WIN_10: WordPad
GROUP_WIN_11: WordPad
GROUP_MAC: TextEdit
GROUP_LINUX: gedit
Install
contextreset.zip ldml keyboard
TEST_CONTROL_1
Expected result:
ê
This was previously failing
TEST_FRAME_KEY_RESET_MARKERS
Expected result:
e
TEST_FRAME_KEY_RESET_NO_MARKERS
Expected result:
Star
TEST_FRAME_SCROLL_LOCK_NO_RESET
Expected result:
ê
TEST_MODIFIER_TAP_NO_RESET
Do not reset for modifier tap
Expected result:
ê