keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
372 stars 102 forks source link

feat(web): delayed modipress completion 🐵 #9973

Closed jahorton closed 5 months ago

jahorton commented 6 months ago

This aims to cover a few "fun" edge cases that can easily arise during quick typing that involves modipresses:

  1. For longpresses triggered during a modipress that has since been released, the underlying layer for the OSK will not revert until subkey-selection is complete.
    • It felt "weird" when it would swap back early, underneath the subkey menu.
    • Granted, it also feels weird to me when it stays in place, but it's a better representation of the system's state.
  2. For flicks triggered during a modipress that has since been released, the underlying layer for the OSK will not revert until the flick is complete.
    • Note: flick completion may be triggered if a new, incoming tap is received. In such a case:
      1. The flick completes as normal, for the original key from the modipressed layer.
      2. The modipress is then fully resolved, including reversion of the layer to the one that was originally active before the modipress
      3. And then the new, incoming tap is processed - for the pre-modipress layer.

The main idea behind point 2 above: during quick typing, it's very natural to lift the modipress finger to prepare to type a new key. This could easily trigger earlier than intended, so we keep the modipress active while the flick is still active.

Point 1 above then provides consistency with point 2 - in both cases, the layer "sticks" until the pending gesture is completed.

In case the user accidentally presses a new key either slightly before or simultaneously with release during this state, we can still allow the flick to complete. Since this is generally during quick typing, and the user did lift the modipress key, they've likely has anticipated the layer to return - and so we adjust for that expectation.

I will say that point 2 above was an interesting niche case to implement - it involved a second coming of the TWo caps problem (#7173 / #9802) from a slightly different angle. While it does add some complexity, it should help make flicks more robust to certain natural 'optimizations' we often subconsciously make while typing quickly.

Notes for future revisitation

❗ ❗ ❗

I ended up throwing in quite a number of other fixes into this one, as it held the big user test suite and was used as the anchor-point for demo builds. Not exactly ideal, but it is what it is at this point. FWIW, I did aim to keep the commits reasonably clean, so going commit by commit might help... though there was a merge from the base (including a master update) that may make certain aspects interesting to follow.

User Testing

Note that most of these tests will be repeats of user tests from previous PRs. Consider this largely a feature-branch regression-test suite... and thus tests worthy of consideration for becoming permanent regression tests once the feature-branch lands.

TEST_10_KEY_ROTA: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Classic 10-key".
  2. Verify that all of the keys numbered 1-9 aside from 7 have three or more lowercase letters as key hints.
  3. Tap the 8 key once, then wait for a second.
  4. Tap the 8 key twice in quick succession, then wait for a second.
    • Expected output: 8a.
  5. Tap the 8 key continuously for at least 2 seconds.
    • it should rotate through: 8a8, 8aa, 8ab, 8ac, 8aA, 8aB, 8aC, then back to 8a8 (and continue from there)
    • stop tapping when the new character is a capital letter.
  6. Verify that the key hints are now capitalized.
  7. Double-tap the 9 key once. A D should be emitted as the result.
  8. Repeatedly tap 9 for at least five seconds, paying attention to the key hints.
    • Whenever a lowercase letter is the result, the key hints should be lowercase
    • Whenever an uppercase letter is the result, the key hints should be uppercase

TEST_10_KEY_DIACRITICS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select the text-area element - the first input-accepting element on the page.
    • Placeholder text: "Type here".
    • Is directly underneath "Type in your language in this text area:"
  2. Select "English...", then "Diacritic 10-key Rota".
  3. Tap the '◌̀' key. (The key between backspace [above] and enter [below], representing a diacritic)
  4. Double-tap the 8 key.
    • Expected result: à
  5. Triple-tap the '◌̀' key - a circumflex diacritic should appear over the à.
  6. Triple-tap the 9 key.
    • Expected total result: àê
  7. Tap the '◌̀' key. A diacritic should appear over the ê.
  8. Deselect the text-area element.
  9. Reselect the text-area element, placing the point of text entry (the caret) at the end, after the ê.
    • Note for future test-maintenance: the point is to trigger a context-reset.
  10. Double-tap the 8 key.
    • Expected final result: àềa image
    • For clarity:
      • With a copy of the à's accent-mark on top the original ê, as in the cropped screenshot above.
        • It looks different in raw-text form here due to GitHub's font / rendering.
      • Note: there is no matching accent-mark on the second a.

TEST_APP_10_KEY_DIACRITICS: Using the Keyman for Android test artifact...

  1. Download this KMP: https://jahorton.github.io/diacritic_rota.kmp
  2. Open / install it for use within the Keyman app.
  3. Tap the 7 key twice; verify that 77 is output.
  4. Tap the '◌̀' key. (The key between backspace [above] and enter [below], representing a diacritic)
  5. Double-tap the 8 key.
    • Expected total result: 77à
  6. Triple-tap the '◌̀' key - a circumflex diacritic should appear over the à.
  7. Triple-tap the 9 key.
    • Expected total result: 77àê
  8. Tap the '◌̀' key. A diacritic should appear over the ê.
  9. Open the in-app Settings menu.
    • Note, for test maintenance: the point of this step and the next one is to trigger a context-reset.
  10. Close the in-app Settings menu.
  11. Double-tap the 8 key.
    • Expected final result: 77àềa image
    • For clarity:
      • With a copy of the à's accent-mark on top the original ê, as in the cropped screenshot above.
        • It looks different in raw-text form here due to GitHub's font / rendering.
      • Note: there is no matching accent-mark on the second a.

TEST_BASIC_SIMPLE_SHIFT: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key once.
  4. Verify that the keyboard changes to, and stays on, the shift layer.

TEST_BASIC_MODIPRESS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap and hold the shift key.
  4. Verify that the keyboard changes to the shift layer.
  5. Tap the A key, verifying that it produces an A.
  6. Release the shift key.
  7. Verify that the keyboard changes to the default layer as a result of step 6.
  8. Repeat steps 3 through 7 quickly, holding shift just long enough to tap the shift layer's A key, then releasing quickly.

TEST_BASIC_MODIPRESS_HOLD: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap and hold the shift key.
  4. Verify that the keyboard changes to the shift layer.
  5. Wait at least one second.
  6. Release the shift key.
  7. Verify that the keyboard changes to the default layer as a result of step 6.

TEST_NUMERIC_FROM_SHIFT: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key, making the shift layer active.
  4. Tap and hold the numeric key (123) underneath the shift key.
  5. Verify that the keyboard changes to the numeric layer - numbers in the top row, mathematical symbols elsewhere.
  6. Longpress the % key and select the subkey.
  7. Verify that is emitted as text.
  8. Release the numeric key.
  9. Verify that the keyboard changes back to the shift layer.
  10. Tap the numeric key, releasing it immediately.
  11. Repeat steps 6 and 7, longpressing the % key and select the subkey.
  12. Verify that the keyboard automatically changes back to the default (lowercase) layer.

TEST_DELAYED_SUBKEY: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" (for the SIL EuroLatin keyboard)
  2. Tap the spacebar. (The keyboard should be on the default [lowercase] layer after this.)
  3. Tap the shift key, making the shift layer active.
  4. Tap and hold the numeric key (123) underneath the shift key.
  5. Verify that the keyboard changes to the numeric layer - numbers in the top row, mathematical symbols elsewhere.
  6. Longpress the % key, but do not select a subkey yet.
  7. Release the numeric key.
  8. Verify that the keyboard changes remains on the numeric layer, with the subkey menu still displayed.
  9. Select the subkey.
  10. Verify that is emitted as text and that the layer has changed back to the shift layer.

TEST_DOUBLETAP_CAPS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "Norwegian" - you should then see "Norwegian - EuroLatin (SIL)" in the space bar.
  2. Type All.
  3. Double-tap the shift key. Afterward, the shift key should be highlighted, and its up-arrow should have a thin line underneath, as if underlining it.
  4. Without touching the shift key at any point, type CAPS.
    • If this is impossible - i.e., the layer shifted out from underneath you - FAIL the test.
  5. Single-tap the shift key. Afterward, you should be returned to the base layer.
  6. Type work.
  7. Tap the shift key once.
  8. Without touching the shift key at any point, type Well.
    • If this is impossible - i.e., the layer did not drop the the base layer when expected - FAIL the test.
    • Expected final result, in total: All CAPS work Well

TEST_CUSTOM_MULTITAP_MODIFIER: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the ◌́ key. (The key between the spacebar [left] and the Enter key [right].)
  3. A number of letters, including all of the vowels, should change to versions using the ◌́ accent mark.
  4. Tap the á and verify that the corresponding letter is output.
  5. Release the ◌́ key; you should be returned to the default layer.
  6. Note the hints on the ◌́, then triple-tap it, holding on the third tap.
  7. A number of letters, including all of the vowels, should change to versions using the ◌̂ mark
    • This corresponds to the second diacritic from the ◌́-key's hint.
  8. Tap the â and verify that the corresponding letter is output.
  9. Release the ◌́ key; you should be returned to the default layer.

TEST_ALTERNATING_SHIFT_AND_KEY: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the SHIFT key
  3. Tap the H key and release it.
  4. Release the SHIFT key.
  5. Verify that an H is output and that the layer returns to default after step 4.
  6. Repeat steps 2-5 five times.
  7. Repeat steps 2-4 quickly for at least 10 seconds.
    • Do not bother checking the result layer; the priority is to be quick here.
    • FAIL this test if any output text is replaced or removed during these repetitions.
    • FAIL this test if text (either h or H) is not output each time you press the h / H key.
  8. When done with step 7's repetitions, tap the numeric (123) key and verify that the layer switches properly to keys with numbers in the top row and symbols in the middle two rows.

TEST_FLICKS_BASIC: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. Drag it in an northwest direction (up and to the left) for at least a key-width.
  4. Release it.
  5. Expected result: ù
  6. Tap the u key again and drag it in a northeast direction for at least a key-width before releasing it.
  7. Expected result: ú (does not replace the previous output)
  8. Tap u again and drag north (straight up) in the same manner.
  9. Expected result: û.

TEST_FLICK_CORRECTION: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. Drag it in an purely west direction (straight left).
  4. Release it.
  5. Expected result: ù
  6. Tap and hold the u key.
  7. Drag it purely south (straight down).
  8. Release it.
  9. Expected result: u. The flick animation and key preview should disappear.
  10. Tap and hold the u key again.
  11. Drag it up just a few pixels / millimeters - a very short distance - then release it.
  12. Expected output: u.

TEST_FLICK_LOCKING: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the u key.
  3. For the following steps, pay attention to the animated preview.
  4. Drag it purely north (straight up) - you should see the û key in the preview area (assuming it's not blocked by your finger).
    • If the animation was "jumpy" - if there was a lack of motion at the same time you moved your finger, then a big "jump" to catch up - FAIL this test.
    • If any such "jumps" were directly tied to you moving your finger quickly, do not fail this test.
  5. Return your finger to its original position; the 'u' key should slide back into place.
    • Note: "original position" is kind of important; if you return slightly to the left of the original position, the next step may not work well.
  6. Now, drag your finger in a up-and-right motion: you should also see a preview for ú as you do so.
    • Again, if the animation felt "jumpy" and not well connected to your input, FAIL this test.
  7. While the ú is visible and in the center, lift your finger and end the gesture.
  8. Verify that a ú is output.
  9. Repeat steps 2 through 6 for the a key; you should see similar previews and text there. (â, á)

TEST_MODIPRESS_MULTITAP_FLICK_PREVIEW: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Double-tap and hold the SHIFT key, keeping it stationary for all following steps.
  3. Tap and hold the U key.
  4. Verify that the key preview is visible.
  5. Drag the finger on U purely north (straight up) - you should see the Û key in the preview area (assuming it's not blocked by your finger).
  6. Verify that the flick-animation in the preview scrolls to a Û.
  7. With the Û fully in-view, release the flick.
  8. Verify that a Û is output.
  9. Repeat steps 2 through 6 quickly three times, including the "verify" steps.

TEST_FLICK_DURING_MODIPRESS: Using the standard "Test unminified KeymanWeb" test page from a mobile device...

  1. Select "English...", then "Gesture Prototyping".
  2. Tap and hold the SHIFT key, keeping it stationary for all following steps.
  3. Tap and hold the U key.
  4. For the following steps, pay attention to the animated preview.
  5. Drag it purely north (straight up) - you should see the Û key in the preview area (assuming it's not blocked by your finger).
    • If the animation was "jumpy" - if there was a lack of motion at the same time you moved your finger, then a big "jump" to catch up - FAIL this test.
    • If any such "jumps" were directly tied to you moving your finger quickly, do not fail this test.
  6. Release the SHIFT key (held since step 2).
  7. Verify that the shift layer (all uppercase) remains visible and that the flick animation is still active.
  8. Return your finger to its original position; the 'U' key should slide back into place.
  9. Now, drag your finger in a up-and-right motion: you should also see a preview for Ú as you do so.
    • Again, if the animation felt "jumpy" and not well connected to your input, FAIL this test.
  10. While the Ú is visible and in the center, tap on the Q key.
  11. Verify that the keyboard returns to the default (lowercase) layer and that either Ú or Úq is output.
    • Úq is ideal, but if your a tap is quick, you may only get the Ú.

I will note that sometimes, when doing the input quickly, I get only Ú (and not the q). Not sure exactly why that is, but I felt it may be worth noting... and worth a future investigation. It is likely typing-speed related, since there's a fair bit of await-async there.

keymanapp-test-bot[bot] commented 6 months ago

User Test Results

Test specification and instructions

Test Artifacts

bharanidharanj commented 6 months ago

Test Results

..1to9 numbers with key hints

..Tap the 8 key twice in quick succession

..Tap the 8 key continuously for at least 2 seconds

..Repeatedly tap 9 for at least five seconds..

bharanidharanj commented 6 months ago

Test Results

..result 1

..result 2

..result 3

bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago

Test Results

..subkey text 1

..subkey text 2

bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago
bharanidharanj commented 6 months ago

Test Results

bharanidharanj commented 6 months ago

Test Results

jahorton commented 6 months ago

TEST_ALTERNATING_SHIFT_AND_KEY (FAILED): (please wait for the all-in-one retest request)

TEST_CUSTOM_MULTITAP_MODIFIER (FAILED): (please wait for the all-in one retest request)

TEST_APP_10_KEY_DIACRITICS (FAILED): (please wait for the all-in one retest request)

jahorton commented 6 months ago

@keymanapp-test-bot retest all

Just pushed a bunch of fixes and such for the various issues we've been noting - things should work a lot better now, assuming the build goes through.

Since there have been a lot of changes, it's best to be sure I didn't break something that had been working before.

bharanidharanj commented 5 months ago

Test Results

..Expected Output 8a

..Typing 8 key continuously

..Double-tap the 9 key once

..Repeatedly tap 9 key for at least five seconds

bharanidharanj commented 5 months ago

Test Results

..Double tap 8 key

..Triple-tap the 9 key

..Double-tap the 8 key

jahorton commented 5 months ago

Test Results

  • TEST_10_KEY_DIACRITICS (FAILED): [...]

Oh right, we changed that behavior with #9944, and the most recent merge with the base-branch added that in. Backspace doesn't nuke the deadkeys anymore... which means I need to rewrite that test to use a different deadkey-erasing behavior.

jahorton commented 5 months ago

@keymanapp-test-bot retest TEST_10_KEY_DIACRITICS

@bharanidharanj That failed test is on me; I forgot that we changed a related behavior with #9944. I've updated the test instructions accordingly.

Turns out that I discovered a bug on master while doing so; for the sake of user testing, I've gone ahead and fixed it here... and 🍒-picked it as #10039 for master. It'd require a retooling of the test keyboard and heavier retooling of the instructions otherwise.


Also, something probably worth note/making an issue: TEST_APP_10_KEY_DIACRITICS will fail if performed on an iOS device if the 7 key step is removed... if the text area is empty before starting the test. My intuition tells me that this is due to one of the iOS quirks we noticed a while back: the iOS equivalent to a text-services framework signals a context reset any time the active text-input receiver has its text cleared. Well... multitapping on a context-initial diacritic-output-only key probably isn't helping things there, possibly causing a context-desync between the app and the keyboard engine. That doesn't seem entirely right, but it's probably quite related.

As it appears to be due to something iOS-specific and does not occur on Android, I think this issue should be documented and pursued separately.

On Android, the context-initial diacritic doesn't seem to get displayed properly in the emulator setup I use most frequently, but it is still present and does still combine properly according to keyboard rules. (Notably, placeholder text disappears when it should be visible, indicating the presence of text... even if it - the isolated diacritic - is not being displayed properly.) That'd be more of a rendering issue within the app and/or on Android, I guess.

jahorton commented 5 months ago

Also, something probably worth note/making an issue: TEST_APP_10_KEY_DIACRITICS will fail if performed on an iOS device if the 7 key step is removed... if the text area is empty before starting the test. My intuition tells me that this is due to one of the iOS quirks we noticed a while back: the iOS equivalent to a text-services framework signals a context reset any time the active text-input receiver has its text cleared. Well... multitapping on a context-initial diacritic-output-only key probably isn't helping things there, possibly causing a context-desync between the app and the keyboard engine. That doesn't seem entirely right, but it's probably quite related.

As it appears to be due to something iOS-specific and does not occur on Android, I think this issue should be documented and pursued separately.

Eh, why not.

Did a little investigation: our old enemy, U+200E (https://unicode-explorer.com/c/200E) has returned to haunt us. It's being automatically inserted at the start of the context whenever a diacritic is added in that position or directly after a different diacritic meeting this compound condition (e.g., it can chain).

This is occurring via setKeymanVal calls to the ios-host.js script:

https://github.com/keymanapp/keyman/blob/6b8a06a52faa16d3f1d3501ff8ac5ee66f1535fa/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js#L292-L295

This has been verified via breakpoint when performing a repro for the conditions described above - that if-condition will trigger, resulting in, among other things, a deadkey-wipe.

The Swift-side interface that calls this:

https://github.com/keymanapp/keyman/blob/6b8a06a52faa16d3f1d3501ff8ac5ee66f1535fa/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift#L231-L251

Well, huh. That should handle it, right? Well... here's the thing...

Screen Shot 2023-11-22 at 10 27 52 AM

Screen Shot 2023-11-22 at 10 26 16 AM

So... there's no match, the byte-header doesn't get filtered, do not pass Go, do not collect $200, etc. Of course, that did work before... so a recent-ish version of iOS must have changed how the byte-header gets encoded. The filter-block was written in #1756 for version 14.0 in May 2019, and there have been quite a few new versions of iOS since then.

Also, I'd noticed weirdness with predictive-text even on master - how an extra backspace was needed after reaching cleared text at the start of context for predictions to work right. It's definitely the same thing - our pred-text engine doesn't filter out byte-order marks either.

jahorton commented 5 months ago

Given the investigation and related behaviors I've noted, it's a bug on the current master and thus should be handled separately.

jahorton commented 5 months ago

And, of course, fixing the fact that context-resets weren't resetting deadkeys (a la #10039)... caused me to run into a different context-sync issue, at least on iOS. Now checking if it also happens on Android.

Core issue there: there are two text updates during a multitap: 'restore original context' and then 'apply new text effects'. If text is updated from the app based on the first and not the second, while the JS side has done both, that causes a context desync.

jahorton commented 5 months ago

As that last comment is something only relevant for feature-gestures - because of multitaps - I went ahead and fixed that here.

bharanidharanj commented 5 months ago

Test Results

..Double-tap 8 key

..Triple-tap 9 key

..Double-tap the 8 key after the change

bharanidharanj commented 5 months ago

Test Results

..Double-tap the 8 key

..Triple-tap the 9-key

..Double-tap the 8th key

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results

..After running step 7

..After running step 12

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results

..H is the output after running step 5

..No deviation in the output after running step 7

..Numeric key appears after running step 8

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results

jahorton commented 5 months ago

Test Results

  • TEST_CUSTOM_MULTITAP_MODIFIER (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. Noticed that after running step 4, it is showing the wrong output in the text screen.

Could you please elaborate on what the error is here? In the screenshot above... isn't that the expected output from the a key from the layer reached by triple-tap?

To be clear - from this layer:

image

... wait a second. Step 4 - the first key. And yep, test-spec error again. That output is correct for the layer you should be reaching. Man, I did not proofread the tests with the varied diacritic forms correctly.

TEST_CUSTOM_MULTITAP_MODIFIER (PASSED): In which I update a test spec again; had the wrong output specified for a step, but the test results were correct for actual (and intended!) behavior.

jahorton commented 5 months ago

Test Results

  • TEST_FLICK_CORRECTION (FAILED): Retested using the standard "Test unminified KeymanWeb" test page from a mobile device (Samsung Galaxy A23 - Android 13) and here is my observation: 1. Selected "Gesture Prototyping" keyboard. 2. After running step 4, it is verified that it outputs ù on the screen. 3. After running step 8, it is observed that the u letter appears on the screen. seems to be an issue. 4. After running step 11, it is verified that it outputs u on the screen.

TEST_FLICK_CORRECTION (PASSED): In which it turns out another test spec needed updating to match current behavior. I think I wrote that test before the locked-flicks stuff was fully implemented; it makes sense for step 8 to emit that 'u', now that I inspect it more closely.

I've gone ahead and adjusted the test-spec accordingly.

bharanidharanj commented 5 months ago

Test Results

..u text animated preview

..a text-animated preview

bharanidharanj commented 5 months ago

Test Results

bharanidharanj commented 5 months ago

Test Results