fix(web): restores compat with older Android devices 🐵 #9943
Conversation
jahorton
requested review from
mcdurdin,
darcywong00 and
sgschantz
as code owners
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
| this.right = x + width; | ||
| }; | ||
|
|
||
| // https://stackoverflow.com/questions/53308396/how-to-polyfill-array-prototype-includes-for-ie8 |
There was a problem hiding this comment.
| // https://stackoverflow.com/questions/53308396/how-to-polyfill-array-prototype-includes-for-ie8 | |
| // https://stackoverflow.com/questions/53308396/how-to-polyfill-array-prototype-includes-for-ie8 | |
| // Needed until Chrome 47 |
| Array.prototype.includes = function(search){ | ||
| return !!~this.indexOf(search); | ||
| } | ||
| } |
There was a problem hiding this comment.
-
Is there any syntax required for this polyfill to work with KeymanWeb's ES Modules?
Lines 95 to 96 in 33eae05
If this is the only KeymanWeb "asset" not copied from KeymanWeb, I'd probably add a comment about "other-polyfills.js" not needing to be copied.
There was a problem hiding this comment.
other-polyfills.js is hand-maintained, not being copied from a Node package. The only "copying" done here was copying the Array.prototype.includes polyfill used by the lm-worker into other-polyfills.js.
|
Will this fix #9561? |
I did nothing to target #9561, but it didn't show up, even when I tried looking for it a bit. It has been noted to be "on again", "off again" in some sense... I'll try to keep an eye out for the issue arising again and do a similar investigation if and when it does occur. ... I suppose I could try creating an alt-repo, checking out the version-tag in the report, and locally rebuilding in a repo attempt. Maybe it'll turn something up quickly. |
|
... of course I didn't see it - I've been doing Manually editing the |
| @@ -34,7 +34,7 @@ if(MINIFY) { | |||
| sourcemap: 'external', | |||
| sourcesContent: DEBUG, | |||
| minify: true, | |||
| keepNames: true, | |||
| keepNames: false, // Do NOT enable - will break under Android 5.0 / Chrome 35 environments! | |||
There was a problem hiding this comment.
An update thanks to evanw/esbuild#3477 (comment): this is likely a non-issue starting in Chrome 43.
https://caniuse.com/mdn-javascript_builtins_function_name_configurable_true
Could you update the description to describe the problem in more detail? |
Fixes #9443.
Turns out that the
es6-shimpolyfill doesn't actually polyfill quite everything we want:Object.values,Object.entries- requires Chrome 54. Fortunately, it's also very easy to workaround with minimal code restructuring.Array.includes: Chrome 47.DOMRectwith constructor: Chrome 61.getBoundingClientRectgoes as far back as Chrome 18; the gesture engine has components built on this method in particular.Also disables
keepNamesin the lm-worker's es-build config, as noted here: #9943 (comment)@keymanapp-test-bot skip