borsini / chrome-otto-tabs

Smart tabs management
https://chrome.google.com/webstore/detail/otto-tabs/pjgajilkdijnbfmglfbpnenocpajmdlb
30 stars 5 forks source link

Migrate manifest v2 -> v3 #13

Closed djdv closed 1 year ago

djdv commented 1 year ago

Loading this extension as-is throws a warning about v2's deprecation. This is in preparation for: https://developer.chrome.com/docs/extensions/mv3/mv2-sunset/ Following: https://developer.chrome.com/docs/extensions/mv3/mv3-migration/#man-sw

The extension's version number should probably be incremented as well. (Either in this commit or one directly following it.)

I'm running this on Vivalid and it seems to work (when on top of https://github.com/borsini/chrome-otto-tabs/pull/11), but otherwise this is not extensively tested.

fixes #7 fixes #16


Extra note: It doesn't seem required, but it might be better to move background.js's content to another file. Then change bacground.js to something that looks like the (now deleted) background.html page, except using importScripts instead of <script> tags. As-is, background.ts seems to import ./tabs_helpers.js anyway/explicitly so it doesn't seem to make a difference when iusing "type": "module".

djdv commented 1 year ago

I rebased this on master and incorporated the suggestions. I'm curious if I need to add type="module" for localise.js or if it's okay as-is. This browser is configured for English so I might not be hitting those paths.

When loading the extension unpacked it seems to work and I don't see any immediate errors. However, I'm experiencing an issue when trying to load it as a zip. Vivaldi complains about

Localization used, but default_locale wasn't specified in the manifest.

But that's clearly not the case. It's declared and defined as en in the manifest file, so I'm not sure what's causing that. Could you test this on your machine? It could be a bug in the nightly build I'm using, or something to do with the build process. I'm on Windows and yarn archive doesn't handle the move or zip operations correctly, so I do them manually which could be messing something up.

Edit: Accidentally committed a line ending change; force pushed over it https://github.com/borsini/chrome-otto-tabs/compare/fb7ae9b4968a0e02c15e01e53f39dc9b7d7f8cc5..703d0d1e48b1b422d50df14d222eab57018b63e2

borsini commented 1 year ago

I rebased this on master and incorporated the suggestions. I'm curious if I need to add type="module" for localise.js or if it's okay as-is.

Localisation seems to work fine without type=module, I'm no js expert but I guess it due to the fact that no imports are done in this file.

This browser is configured for English so I might not be hitting those paths.

As there are no defaults in the html file, if you see the english version, it means that the localisation script was executed and works.

When loading the extension unpacked it seems to work and I don't see any immediate errors. However, I'm experiencing an issue when trying to load it as a zip.

I've never tested importing a zip file. How do you do that? Documentation states that you should Find the extension folder in your File Manager (if you have it as a .zip file, extract it first)

Unpacked works fine on my browsers Vivaldi 6.1.3035.204 (Stable channel) and Chrome 115.0.5790.114 (Official Build)

Edit: Accidentally committed a line ending change; force pushed over it https://github.com/borsini/chrome-otto-tabs/compare/fb7ae9b4968a0e02c15e01e53f39dc9b7d7f8cc5..703d0d1e48b1b422d50df14d222eab57018b63e2

Don't mind, it is just a pet project 😉. Feel free to add as many commits as you like. Once merged everything will be squashed.

djdv commented 1 year ago

Localisation seems to work fine without type=module localisation script was executed and works

Nice. Thanks for double checking it. :^]

I'm no js expert but I guess it due to the fact that no imports are done in this file.

Same here, I'm not super familiar with TS or JS but can usually manage when I need to interact with it 😆. That module explanation makes sense to me.

I've never tested importing a zip file. How do you do that?

Whoops, I think I was mistaken. I assumed the zip produced by yarn archive would output a file intended for the browser to load.

In Vivaldi (maybe Chrome as well) if you're on the extension page with developer mode enabled (have to refresh if toggling it on for the first time) you can drag the zip file into the page and it will try to load it. If I use the "pack extension" button to create a signed crx file instead, that seems to work as intended.

Don't mind, it is just a pet project 😉. Feel free to add as many commits as you like. Once merged everything will be squashed.

Sounds good. :^] If this is working there probably won't be anything more to add unless someone more familiar with the manifest spec chimes in with requests for changes.

That said, it might be best to hold off until the browser/market vendors actually start requiring v3. That way the extension doesn't prematurely start excluding browsers that only support v2 at the moment. Your call on that one.

borsini commented 1 year ago

I assumed the zip produced by yarn archive would output a file intended for the browser to load.

I added documentation to help understand the purpose of each command

That said, it might be best to hold off until the browser/market vendors actually start requiring v3

Chrome 88 was released on Jan 20, 2021, I guess it's totally safe to merge this one. Thanks for your contribution 💪

borsini commented 1 year ago

However, I'm experiencing an issue when trying to load it as a zip. Vivaldi complains about Localization used, but default_locale wasn't specified in the manifest.

0158c7629bedd033e0cc8265c32e55844a1e0de7 fixes the archive command and gets rid of the error you had 😉