ff2mpv
ff2mpv copied to clipboard
woodruffw
experimental port to mv3
Awesome, thanks for exploring this. We'll leave it open as a reference for the eventual migration.
Thanks for rebasing. Let me know when you think this is ready for a full review, and I'll do so.
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.
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 🙂
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.
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.
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.
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().
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.
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.
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.
@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
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.
Sorry I can't work on it right now. BTRFS decided to corrupt my root filesystem.
No hurries on this. Do let me know if this presents issues that you'll like me to take a look.