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

Migrate manifest v2 -> v3 #13

Merged
merged 4 commits into from Aug 6, 2023
Merged

Migrate manifest v2 -> v3 #13

merged 4 commits into from Aug 6, 2023

Conversation

djdv
Copy link
Contributor

@djdv djdv commented Jan 14, 2023

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 #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".

Copy link
Owner

@borsini borsini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this PR 🙏
I did some tests and there is an error when showing the popup window.
Capture d’écran 2023-08-02 à 22 43 48

It can be fixed easily by adding type="module" to <script src='./popup.js'></script>

static/manifest.json Outdated Show resolved Hide resolved
@djdv
Copy link
Contributor Author

djdv commented Aug 3, 2023

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
Copy link
Owner

borsini commented Aug 4, 2023

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
Copy link
Contributor Author

djdv commented Aug 4, 2023

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
Copy link
Owner

borsini commented Aug 6, 2023

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 borsini merged commit 2e626bd into borsini:master Aug 6, 2023
1 check passed
@borsini
Copy link
Owner

borsini commented Aug 14, 2023

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.

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

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

Successfully merging this pull request may close these issues.

Saving the settings Tabs in stacks closing on browser restart if more than 5 [Vivaldi]
2 participants