keyman icon indicating copy to clipboard operation
keyman copied to clipboard

bug(web): internal null error occurred during caps multitap

Open jahorton opened this issue 2 years ago • 3 comments

Describe the bug

While trying to identify an older test page useful for testing a currently-active PR, I happened upon this error when using web/testing/caps-lock-layer-3620/:

image

Note the floating undefined in the source area - ruleBehavior itself is undefined, but we're only null-guarding its transcription property here.

The KeyEvent that produced this: I quickly double-tapped the 'shift' key while using a tablet form-factor - either from the default layer or the shift layer.

To Reproduce

  1. Go to the Web testing page web/testing/caps-lock-layer-3620/. ("Test Caps Lock Layer (3620)")
  2. Click on the text area to summon the OSK.
  3. Quickly double-tap the shift key to access the CAPS layer.
  4. Check the JS console
    • Alternatively, if you're doing this in Chrome with its mobile-device emulation mode, Chrome may auto-break when the error occurs! How helpful!

Note that the caps-lock layer still appears... but the first key following the error appears to get "swallowed".

Expected behavior There should be no JS error, nor should the following keystroke be swallowed. I imagine the former causes the latter, but we can't be sure without further investigation.


KeymanWeb:

  • KeymanWeb Version: 15.0.260

Other notes: I seem to recall seeing Sentry errors with a similar error and call-stack. I'll try to find 'em and link them to this issue. Of course, the question there is whether or not this only happens from multitap cases.

jahorton avatar Jun 27 '22 01:06 jahorton

I believe this was fixed in #6789 (15.0.261)

mcdurdin avatar Jun 27 '22 03:06 mcdurdin

I believe this was fixed in #6789 (15.0.261)

For some definitions of "fixed"... but it feels like possibly-bad design for us to intentionally be producing null RuleBehaviors from multitaps. I want to make sure this scenario is actually reasonable.

In other words, does that fix affect the cause, or just an effect of the actual cause?

jahorton avatar Jun 27 '22 03:06 jahorton

The null guard is absolutely 100% necessary because a null RuleBehavior is possible for layer switch keys. The very next line of code in #6789 tests for a null RuleBehavior!

So it does fix the cause. We can argue about whether it's a bad design, but it's your design I think? In any case, any redesign for multitap is part of the gesture work and probably does not need to be tracked as a separate issue.

mcdurdin avatar Jun 27 '22 20:06 mcdurdin

Closing as fixed, some redesign may happen with gestures

mcdurdin avatar Oct 17 '22 05:10 mcdurdin