keymanapp / keyman

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

bug(web): An error message related to the keyboard is displayed on the Android 5.0 OS #9561

Closed bharanidharanj closed 7 months ago

bharanidharanj commented 9 months ago

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 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

darcywong00 commented 9 months ago

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

bharanidharanj commented 8 months ago

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

darcywong00 commented 8 months ago

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 commented 8 months ago

@jahorton this looks like a missing polyfill in lmworker.

jahorton commented 8 months ago

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.

jahorton commented 8 months ago

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

jahorton commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

Bug report submitted @ https://github.com/evanw/esbuild/issues/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 commented 7 months ago

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

jahorton commented 7 months ago

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