experimental port to mv3

[eNV25]

It works on chromium but I haven't tested on firefox. Don't merge.

[eNV25]

https://developer.chrome.com/docs/extensions/mv3/mv3-migration-checklist/

[woodruffw]

Awesome, thanks for exploring this. We'll leave it open as a reference for the eventual migration.

[woodruffw]

Thanks for rebasing. Let me know when you think this is ready for a full review, and I'll do so.

[eNV25]

I don't think it's ready yet. While it works for Chrome, I can't test it properly for Firefox.

The Firefox implementation of MV3 does have support for service workers yet. We should probably wait until that's finished. But I did try out what's implemented so far, it works with only changing the manifest to use "background": { "scripts": ["ff2mpv.js"] }. The new APIs like the chrome.scripting are already implemented.

https://developer.chrome.com/docs/extensions/migrating/mv2-sunset/

Anyway, it doesn't look like Chrome has a plan to stop supporting MV2 anytime soon.

[woodruffw]

It looks like MV2's deprecation is happening semi-rapidly after all:

If I'm reading that right, we'll need to switch to MV3 by June 2024. So we'll probably need to take another look at revitalizing this PR soon 🙂

[eNV25]

The only thing that hasn't been resolved so far upstream is background script vs service worker. Chrome only supports service workers and Firefox only supports background scripts.

If it goes on like this we will need to have different manifest.json for Chrome and Firefox. The only difference is the "background" key.

The extension works equally well with both, so it's not a big problem.

[eNV25]

OK, I made further changes to align to the current mv3 recommendations.

./package-firefox.sh works without further changes to the current code. I tested it with Firefox Developer Edition using Load Temporary Add-on, and there are only 2 warnings. The warnings can probably be ignored since I am following the docs.

Reading manifest: Warning processing background: Error processing background: Property "persistent" is unsupported in Manifest Version 3
Reading manifest: Warning processing background.service_worker: An unexpected property was found in the WebExtension manifest.

./package-chrome.sh chromium will complain about the unsupported keys background.scripts and background.persistent. Just remove those in the local copy and it works without errors.

[eNV25]

About #108, the part in ff2mpv.js will need to be rewritten/rebased to support mv3 too. It should be using chrome.contextMenus.onClicked.addListener(...) instead of chrome.contextMenus.create({ onclick: ... }). The onclick property doesn't work in mv3.

Also this is not exactly my business but I think #108 is over-complicated. I would have just re-used mpv --profile=id, instead of having our own concept of profiles in browser storage.

[eNV25]

Actually the whole implementation probably needs to be re-written for mv3. Because it currently assumes that there is persistent background page.

mv3 extensions use either event-driven service workers or event-driven non-persistent background pages.

chrome.extension.getBackgroundPage() does not work with service workers. And Firefox might delete and reload non-persistent background pages at any time, and I don't know how that interacts with chrome.extension.getBackgroundPage().

[eNV25]

We should probably be using chrome.runtime.sendMessage(). @DanSM-5

[eNV25]

Here's a possible protocol for sendMessage:

"event" mandatory key with the following possible values: "createProfile", "deleteProfile", "updateProfile".

"data" optional event specific data, for "createProfile" and "updateProfile" this is the new profile data.

[eduardo-sm]

We should probably be using chrome.runtime.sendMessage(). @DanSM-5

Oh right, I didn't think about manifest v3 when using getBackgroundPage. I used it because it is easier than chrome.runtime.sendMessage() but this can be changed. I can rework it with your proposed protocol.

Overall I need to take a look at the other mv3 specifics:

• chrome.contextMenus.onClicked.addListener()
• chrome.runtime.sendMessage()

Am I missing anything?

Also this is not exactly my business but I think https://github.com/woodruffw/ff2mpv/pull/108 is over-complicated. I would have just re-used mpv --profile=id, instead of having our own concept of profiles in browser storage.

I tried to use profiles but you still need to send the profile id and while experimenting with it I found more convenient to store the mpv flags. If just limiting to profileId, the logic would be very similar but limited to that specific flag. Using browser sync has the additional benefit that if you use the same account, your configurations are sync and will work (I have to deal with multiple computers, so this helps me). The other part is that you still need to keep opening mpv.conf every time you need a change which is what I originally wanted to avoid.

[eNV25]

Once you use the correct APIs that I mentioned in options.js and ff2mpv.js, I can rebase my work here on top of that. I want to avoid touching options code too much.

[eNV25]

@DanSM-5 Please take a look at https://github.com/eNV25/ff2mpv/blob/pr/mv3/ff2mpv.js. In mv3 all logic should run in event listeners, and all event listeners should be registered in the top level.

In particular I want you to see you the initial contextMenus are created at runtime.onInstalled. Do you think this will work with your options logic?

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/create

[DanSM-5]

Sorry for the delay on this.

I have created a PR to your branch @eNV25: https://github.com/eNV25/ff2mpv/pull/1

It is working fine on my side.

[eNV25]

Sorry I can't work on it right now. BTRFS decided to corrupt my root filesystem.

[DanSM-5]

No hurries on this. Do let me know if this presents issues that you'll like me to take a look.

[eNV25]

@DanSM-5 I incorporated your changes, but I wasn't able to properly merge your PR. Moved things around to make the diff look better.

I works according to my testing, please check on your end as well.

[DanSM-5]

@eNV25 I see, yesterday I only finished doing the merging with upstream but didn't had a chance to test. I was planning to do that today but I got a bit busy. I'll test this tomorrow.

[eNV25]

@DanSM-5

* Not sure if this happens to you but in firefox when loading the extension in **about:debugging** it looks like `chrome.runtime.onInstalled.addListener` is not firing for me. I've added logs but those are not recorded and as a result the context menu doesn't work. It works fine if the content is moved to a function that is called when the script runs (similar to how mv2 works)

Works on my machine. Firefox Developer Edition (basically Firefox Beta). All context menus work and update correctly.

The local storage is empty for temporary add-ons, maybe that's what you are seeing? Otherwise have you tried to delete the normal add-on before installing the temporary one?

[DanSM-5]

The local storage is empty for temporary add-ons, maybe that's what you are seeing? Otherwise have you tried to delete the normal add-on before installing the temporary one?

Seems like the issue was not removing the normal ff2mpv. Now it is loading fine.

[eNV25]

For a while I've been seeing an issue where the "Play in MPV" submenu, doesn't appear when you first create a profile. Re-creating the profile seems to fix the issue.

[DanSM-5]

@eNV25 did it happened to you after the last changes I made?

I noticed the same when testing the suppression of the error runtime.lastError and I figure it is because the method remove and removeAll are asynchronous. It should work now that that is being awaited. I didn't get the error again after.

[eNV25]

@DanSM-5 I added await to all contextMenu related function calls, and it seems to work now.