Update composekey to manifest v3 #152
Conversation
|
Attempt at a cleanup and submission of the manifest v3 fixups discussed in #147 Note that I'm very much not a chrome extension or even JavaScript person. Review carefully. |
Derived from original work by @gutschke on github (see google#147). Use new storage API. Fix up situation where the callbacks can be invoked in an environment where KeyboardEvent isn't defined (and it can't detect the "location" correctly).
jpalmer
left a comment
jpalmer
left a comment
There was a problem hiding this comment.
Thanks for sending this through, just a few small comments
|
|
||
| if (key == 'Enter' | ||
| && location == KeyboardEvent.DOM_KEY_LOCATION_NUMPAD) { | ||
| && location && location == KeyboardEvent.DOM_KEY_LOCATION_NUMPAD) { |
There was a problem hiding this comment.
you shouldn't need location && location == something
location == something should be fine
(and === would probably be better)
There was a problem hiding this comment.
This turned out to be a special case of the KeyboardEvent-missing problem, testing location was a proxy.
|
|
||
| let location = keyData.location; | ||
| if (location === undefined) { | ||
| if (location === undefined && typeof KeyboardEvent !== 'undefined') { |
There was a problem hiding this comment.
This extra check seems odd to me - could you add a comment to explain why we need it
There was a problem hiding this comment.
What's happening is that "KeyboardEvent" is an undefined symbol in some contexts where this gets run. I don't remotely understand the service worker architecture to understand why that might be, but it seems not to correspond to actual user key events.
There was a problem hiding this comment.
I think there is no DOM in the serviceworker environment which might explain why there is no keyboardevent - but in that case I think we can assume that keyboardevent would always be null?
| setComposeFile(defaultComposeFile); | ||
| } else { | ||
| console.log('Loaded compose file from local storage (%d characters).', | ||
| lengthInCodepoints(content)); |
There was a problem hiding this comment.
should this content be val?
There was a problem hiding this comment.
Indeed. And indeed the custom compose file code path (which I didn't try)0 isn't working, for this bug for sure but there are other exceptions I need to look at. Update coming, though ideally someone with a better idea of this stuff might want to try too.
|
Also, just checking, but did you test this locally to see if it all works as expected? |
|
Indeed, this isn't ready. The code in options.js expects to use a background page for storing the text of the custom .XCompose file (I'm not entirely sure why? It's just for the options UI and ephemeral, right?), and background pages don't work anymore as I'm gathering. There's some support in there for using the option page DOM instead, but turning that on doesn't work and I get oddball serialization output instead of the file contents. This may need someone with more web-fu that I have. I do firmware. But I'll continue to work on it time available, as I really genuinely miss this extension badly. |
|
Thank you for working on this, I really miss this working properly also. If I had the skills I would jump in |
|
Note: if you enable chrome://flags/#allow-legacy-mv2-extensions, and enable developer mode on chrome://extensions you can then still load the ComposeKey manifest as an unpacked extension.
|
|
thanks, that's working for now
…On Wed, Jul 16, 2025 at 2:17 PM Colm ***@***.***> wrote:
*colmbuckley* left a comment (google/extra-keyboards-for-chrome-os#152)
<#152 (comment)>
Note: if you enable #allow-legacy-mv2-extensions, and enable developer
mode on extensions you can then still load the ComposeKey manifest as an
unpacked extension.
Thank you for working on this, I really miss this working properly also.
If I had the skills I would jump in
—
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APYTFEWMZARH3VDBJU6EAQT3I2JMJAVCNFSM6AAAAACASCXRBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTANZZG42TKNJTGA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Is chrome://flags/#allow-legacy-mv2-extensions still supposed to work? (I can't find it). Otherwise does this PR actually work (even if not all functionality is present), should I give it a try? ok, replying to self, it does work but changing settings doesn't. I've hardcoded my preferred composekey in the extension and it works (search for AltRight in background.js) |
5 participants
Derived from original work by @gutschke on github (see #147).
Use new storage API. Fix up situation where the callbacks can be invoked in an environment where KeyboardEvent isn't defined (and it can't detect the "location" correctly).