Skip to content
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

bug(android): embedded popup key issues #6978

Closed
sentry-io bot opened this issue Jul 21, 2022 · 10 comments · Fixed by #7388
Closed

bug(android): embedded popup key issues #6978

sentry-io bot opened this issue Jul 21, 2022 · 10 comments · Fixed by #7388

Comments

@sentry-io
Copy link

sentry-io bot commented Jul 21, 2022

Sentry Issue: KEYMAN-WEB-47

Error:  No base key exists for the subkey being executed: 'default-U_0259'
  at reduceConsoleArgs (/data/user/0/com.tavultesoft.kmapro/app_data/keyman-sentry.js:889:36)
  at console.reportingConsoleWarn [as warn] (/data/user/0/com.tavultesoft.kmapro/app_data/keyman-sentry.js:876:47)
  at g.executePopupKey (source/kmwembedded.ts:391:9)
  at executePopupKey (/data/user/0/com.tavultesoft.kmapro/app_data/android-host.js:352:25)
  at None (<anonymous>:1:1)

Note: while this error only just started showing up with the currently-latest release build... that's only because we just started Sentry-logging console.warn statements. It's clearly an older issue that only just got brought to light because of #6904.

Examining the events of this issue, none of them appear to include the popup- component expected for Web-based popup key IDs... though a comment within the function notes that they're unlikely to be present in this context.

@jahorton
Copy link
Contributor

jahorton commented Jul 21, 2022

Not as frequent, but possibly related:

Sentry Issue: KEYMAN-WEB-48

Error:  Could not find subkey 'U_013A' under base key 'K_S'!
  at reduceConsoleArgs (/data/user/0/com.tavultesoft.kmapro/app_data/keyman-sentry.js:889:36)
  at console.reportingConsoleError [as error] (/data/user/0/com.tavultesoft.kmapro/app_data/keyman-sentry.js:865:49)
  at resolve (source/osk/embedded/subkeyDelegator.ts:73:13)
  at g.executePopupKey (source/kmwembedded.ts:388:9)
  at executePopupKey (/data/user/0/com.tavultesoft.kmapro/app_data/android-host.js:352:25)
...
(1 additional frame(s) were not displayed)

This bug is noted as happening exclusively on Android; we have cases where the subkey passed the check that produces the event seen in the main description, but fails another, later check instead. This appears to be less frequent, but still notable.

@jahorton jahorton changed the title bug(android): mystery attempts to execute regular keys as popup keys. bug(android): embedded popup key issues Jul 21, 2022
@mcdurdin mcdurdin added this to the A16S7 milestone Jul 21, 2022
@mcdurdin
Copy link
Member

Note: #6981 does not fix this.

@mcdurdin mcdurdin modified the milestones: A16S7, A16S8 Aug 7, 2022
@mcdurdin mcdurdin modified the milestones: A16S8, A16S9 Aug 19, 2022
@mcdurdin mcdurdin modified the milestones: A16S9, A16S10 Sep 5, 2022
@darcywong00
Copy link
Contributor

On reviewing Sentry, I'm noting some of the base keys which seem curious

default-K_*Ctrl*
default-K_*Alt*
default-T_TH

(maybe this is from https://github.com/keymanapp/keyboards/blob/cd09a57650dc1a7fc43a287a5910c121a2582e30/release/i/inuktitut_naqittaut/source/inuktitut_naqittaut.keyman-touch-layout#L42-L48)?

@mcdurdin
Copy link
Member

default-K_*Ctrl*
default-K_*Alt*

These two should be impossible to generate, so that smells like something buggy in Keyman.

@darcywong00
Copy link
Contributor

darcywong00 commented Sep 29, 2022

Still trying to get a repro. I got this warning message with english_shavian_qwerty keyboard

No base key exists for latin-default-U_011D+default

No base key exists for shift-U_005C+default

No base key exists for shift-U_2031+default


While mashing layer switching and long-pressing, I also saw warnings:

Potential fat-finger key could not be found in layer!

Perhaps there's a lag in processing longpresses when a layer change has already happened?

@darcywong00
Copy link
Contributor

Here's the steps I had to do to repro on latest master with default sil_euro_latin
I tried to capture on video, but it doesn't show the left thumb tap (on the shift button immediately after starting the long-press on G

Needs a physical device for multi-touch (two thumbs)

  1. Start on the Shift layer
  2. With the right thumb, start long-press on G and hold. Immediately with the left thumb press and release the shift button to switch to Default layer
  3. With the right thumb still held, observe the layer is default layer, but eventually the long-presses for uppercase-G appear.
  4. With the right thumb still held, select one of the long-press options and release
  5. The breakpoint is the Sentry warning message being logged.
basekey-2.mp4

@darcywong00
Copy link
Contributor

I suspect this is a race condition cause there's the Keyman Android Engine has a long-press delay to display the long-press keys, but the layer has since switched...

@darcywong00
Copy link
Contributor

From @mcdurdin

Cancel any pending longpress when a second touch is made.

@jahorton
Copy link
Contributor

jahorton commented Oct 6, 2022

I can still make this one with a very niche scenario:

  1. Press and hold a longpress key
  2. Watch as the subkey menu appears
  3. Quick-tap the globe key
  4. Select subkey (from previous keyboard - it's not cleared!)
  5. Error!

https://sentry.io/organizations/keyman/issues/3410969679/events/5d34ae0c546f405f8b1b82c386e1f74c/

It's pretty niche though, so probably not worth reopening the issue.

Copy link
Author

sentry-io bot commented May 31, 2024

Sentry Issue: KEYMAN-WEB-KJ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants