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(web): An error message related to the keyboard is displayed on the Android 5.0 OS #9561

Closed
1 of 8 tasks
bharanidharanj opened this issue Sep 14, 2023 · 11 comments · Fixed by #9943
Closed
1 of 8 tasks
Assignees
Milestone

Comments

@bharanidharanj
Copy link

bharanidharanj commented Sep 14, 2023

Describe the bug

After the installation or modification of a keyboard, an error message was displayed on the screen.

Reproduce the bug

  1. Install Keyman 17.0.173 build.
  2. Open Keyman In-App.

Noticed that an error message was displayed on the screen.

  1. Download and Install any keyboard.

Here, I observed that once more, an error message appeared on the screen.

  1. Long press the globe key.

We may notice that the keyboard picker menu appears on the screen.

  1. Change the keyboard to a Khmer Angkor keyboard.

Once more, the error message was shown on the screen.

I have attached the screenshot for reference.

Expected behavior

It should not show any error messages following the installation or replacement of a keyboard.

Related issues

9401, 9546

Keyman apps

  • Keyman for Android
  • Keyman for iPhone and iPad
  • Keyman for Linux
  • Keyman for macOS
  • Keyman for Windows
  • Keyman Developer
  • KeymanWeb
  • Other - give details at bottom of form

Keyman version

17.0.173-alpha

Operating system

Android 5.0

Device

No response

Target application

No response

Browser

No response

Keyboard name

No response

Keyboard version

No response

Language name

No response

Additional context

No response

@mcdurdin mcdurdin added auto and removed bug labels Sep 15, 2023
@keymanapp-test-bot keymanapp-test-bot bot added bug and removed auto labels Sep 15, 2023
@mcdurdin mcdurdin added auto and removed bug labels Sep 15, 2023
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S21 milestone Sep 15, 2023
@mcdurdin mcdurdin reopened this Sep 15, 2023
@mcdurdin mcdurdin removed this from the A17S21 milestone Sep 15, 2023
@darcywong00 darcywong00 added this to the A17S23 milestone Sep 25, 2023
@darcywong00
Copy link
Contributor

@bharanidharanj - I cannot repro on the current 17.0.183 alpha branch. Can you retest?

@bharanidharanj
Copy link
Author

@darcywong00 Retested this issue with the latest 17.0.186-alpha build and I am able to reproduce it.

@mcdurdin mcdurdin modified the milestones: A17S23, A17S24 Oct 15, 2023
@darcywong00
Copy link
Contributor

darcywong00 commented Oct 26, 2023

Repro with 17.0.198 alpha and corresponding Sentry report

Cannot redefine property: name
blob:null/8945bc8a-3e0a-48ae-bba8-4b4d29bf8a5a at line 1:69

@mcdurdin
Copy link
Member

@jahorton this looks like a missing polyfill in lmworker.

@mcdurdin mcdurdin changed the title bug(android): An error message related to the keyboard is displayed on the Android 5.0 OS bug(web): An error message related to the keyboard is displayed on the Android 5.0 OS Oct 26, 2023
@mcdurdin mcdurdin assigned jahorton and unassigned darcywong00 Oct 26, 2023
@jahorton
Copy link
Contributor

jahorton commented Oct 26, 2023

Ugh. That's a really uninformative error; the reported location of the error is in the minifying code, not our own. It's not a polyfill; if anything... it's more as if it's an error from trying to polyfill something.

From the compiled, polyfilled webworker - the very start of it:

"use strict";var ae=Object.defineProperty;var v=function(r,e){return ae(r,"name",{value:e,configurable:!0})};

After which follows the body of our first polyfill. But the error happens within the function assigned to v... which appears to be used by esbuild to set properties on objects by the minified bundle.

The oldest release with a related event, as tagged by that Sentry link, is 17.0.154-alpha. Interestingly, it seems to come and go - it's skipped several versions and come back again based on the event logs. Also, 17.0.154 is when #9409 landed... which matches the previous time we had to update for compatibility with the older Android devices. Yaaaaaay.

@mcdurdin mcdurdin modified the milestones: A17S24, A17S25 Oct 27, 2023
@jahorton
Copy link
Contributor

jahorton commented Nov 2, 2023

Tried making a local build and installing it in an emulated Android 5.0 / API 21 device... no errors. Huh.

@jahorton
Copy link
Contributor

jahorton commented Nov 6, 2023

Important: the standard --debug Android build does not use the minified version of KeymanWeb; it's something in the minification that's triggering the issue. Be sure to edit KMEA's build.sh to use Web's app/webview release products [when attempting a local reproduction]!

It's falling over when processing the first polyfill - String.prototype.codePointAt.

And I can manually reproduce it with a rather simple setup, even in the main, es6-shim polyfilled thread:

var temp = function() { console.log("hello") };
console.log(temp.name); // outputs ""
Object.defineProperty(temp, 'name', {value: 'temp', configurable: true}); // will throw the same error.

Try the same in modern Chrome, repeating the second line after the third, and you'll get "temp".

This appears to be something esbuild minification is doing; I'll see if it can be turned off separately from everything else. If not, we may have to disable worker "name" minification, as that would obviously be the minification category it falls under.

@mcdurdin
Copy link
Member

mcdurdin commented Nov 6, 2023

Be sure to edit KMEA's build.sh to use Web's app/webview release products!

Who are you asking to do this? Can you please elucidate and open a separate issue on Android if this is something that needs to be done.

as that would obviously be the minification category it falls under.

I'm not seeing the obviousness here. I think I can work out the dots you've connected but it's far from obvious! Can you be a bit more explicit on the root cause?

@jahorton
Copy link
Contributor

jahorton commented Nov 6, 2023

Ah, on that - that's to pull off a consistent repro. Obviously, don't do this for actual production.

Bug report submitted @ evanw/esbuild#3477.

The role of the responsible function, generated during minification, is to assign each minified function its original, non-minified name as its .name property, a field/property all functions have in modern JS. But, Chrome 35 is so old that the property is something it considers read-only. Hence the error.

And a little more tracing past that: we cannot utilize esbuild's keepNames option. That was the culprit - keepNames: true.

@jahorton
Copy link
Contributor

jahorton commented Nov 6, 2023

I have a fix up as a commit within #9943. If desired, I can also 🍒-pick it easily to master.

@mcdurdin mcdurdin modified the milestones: A17S25, A17S26 Nov 13, 2023
@jahorton jahorton linked a pull request Nov 13, 2023 that will close this issue
@jahorton
Copy link
Contributor

As this has been merged to the feature-gestures branch, I'm going ahead and closing this.

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

Successfully merging a pull request may close this issue.

4 participants