-
Notifications
You must be signed in to change notification settings - Fork 53
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
Use browser-polyfill.js to allow use with chromium #69
Conversation
Very cool, thanks for looking into this. I left a few questions/nitpicks above, but overall this LGTM. |
cc @DanSM-5: how do you feel about unifying on a single port of |
Something to note is Chrome's switch to manifest v3 and the manifest v2 support timeline. This extension uses manifest v2. https://developer.chrome.com/docs/extensions/mv3/mv2-sunset/ It looks like after this month new mv2 extensions cannot be added to the Chrome Web Store, but old ones can be updated for a year. So the best bet for getting this in the Chrome Web Store in this time frame is updating @DanSM-5 fork with this version. The long term strategy for supporting Chrome based browsers should be adopting manifest v3. But this will probably require some changes to the code (eg. how the extension finds the video from the current page). I will probably try to experiment with it at some point in the future. AFAIK Firefox will keep supporting Manifest v2 and also support Manifest v3 in the future for Chrome compatibility. So even if the extension doesn't adopt manifest v3 it should still work with Firefox. Also Firefox nightly does not yet support Manifest v3. |
Thanks for noting that. Yeah, in that case, unifying with the existing Chrome extension makes sense. That will require @DanSM-5 to either become the release manager for Chrome or for them to give me their current signing keys, in which case I don't mind taking over the responsibility. And yeah, we should eventually switch to Manifest v3 overall. I think we can safely earmark that for the point in time when Firefox ESR supports it. |
I think it would be great to unify the development. I don't mind giving you the signing keys @woodruffw. I published it to the chrome store just to avoid updating the app id every time. |
Awesome! We can arrange some kind of secure channel for the keys. Does Keybase or email + PGP work for you? |
I'm not familiar with keybase. Do you recommend it? I can encrypt it with PGP and send it over email. |
Only if you don’t already have PGP set up! Email is good otherwise; my key ID is 0x85ae00c504833b3c.
Sent from mobile. Please excuse my brevity.
… On Jan 14, 2022, at 6:48 PM, DanSM-5 ***@***.***> wrote:
I'm not familiar with keybase. Do you recommend it? I can encrypt it with PGP and send it over email.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.
|
All set, let me know if you received it. |
Received, thank you! I’ll try and decrypt this evening or tomorrow.
Sent from mobile. Please excuse my brevity.
… On Jan 15, 2022, at 12:54 AM, DanSM-5 ***@***.***> wrote:
All set, let me know if you received it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.
|
Just to understand what is going, why is the private key being transferred? As far as I understand it would only allow you to locally sign the extension while having the same extension id as @DanSM-5 's fork. I don't think it allows you to publish to the chrome webstore. https://stackoverflow.com/questions/5604491/how-do-i-transfer-the-ownership-of-a-chrome-extension According to this Stack Overflow to transfer an extension, you need to use a form and have a valid destination account that has paid the $5 fee. https://support.google.com/chrome_webstore/contact/dev_account_transfer But it looks like you can also set up group publishing so multiple devs can manage one extension. https://developer.chrome.com/docs/webstore/group-publishers/ Maybe that is also a good option? |
Oh, I badly misunderstood then -- I thought they key was needed for publishing to the webstore (I've never done any Chrome extension publishing/management before). Sorry for the confusion. Group publishing looks like it's the right approach, although it's weirdly baked into Google Groups. I could create one (I have a G Suite account) if that works for you @DanSM-5, unless you'd prefer to transfer it outright. |
I was confused as well. I didn't know about the limitations of the Chrome extension store. Although I think the private key will be helpful if there is a plan to provide signed packages (crx) directly from the releases. @woodruffw I agree with the group publishing. You can add me to a group and I will add the extension to it. Just want to mention this, it looks like there is a limit per account to create a single group publishing in the chrome store.
If you don't want to use your quota on this, I can volunteer keep updating the extension in the chrome store once this PR gets merged. |
Hooray for arbitrary restrictions! I probably won't ever write another extension, so I don't mind burning my quota on this. I'll set up the group and add you to it, so that there's an extra layer of redundancy for getting releases made for Chrome. |
Okay, I've created the group and sent you an invite to it. Let me know if you have any problems with the invite. |
@woodruffw I waited all day but I didn't received any invitation to join. Could you try to add me again? |
That's odd; it shows you as a member currently. I'll send you an email with a screenshot of what I'm seeing and we'll try to resolve over email (I don't want to disclose your email address or full name on GitHub without your permission, since they aren't immediately visible on your profile.) |
My bad, I am part of the group but I didn't get any notification about it. The group only appears if I try to make a transfer for the extension. I moved the extension to the group. You should have access to it now. |
No problem; everything about this is Google's fault 😉 Yep, I also just confirmed that I have access to the extension. Now that that's all settled, I'll do another review pass of this PR and, with any luck, begin cutting release for both browsers this week! |
I will add a commit with a external manifest json with the extension id already added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Once https://github.com/woodruffw/ff2mpv/pull/69/files#r790180598 is in, I'll merge this and cut a release for both platforms.
Thanks for all of the hard work here, and thanks again @DanSM-5 for the coordination.
Trying things out locally now. It looks like |
Works locally for me! I'm going to merge, do a small amount of clean-up, and cut a new release for both platforms. |
Cut as 4.0.0 and submitted for review to the Chrome Store. Will be published shortly on AMO as well. |
This change adds browser-polyfill.js so that it can also be packaged for chromium. This should result in no change in how the ff2mpv.js is executed on firefox.
More about browser-polyfill: https://github.com/mozilla/webextension-polyfill
./package-chromium.sh
generates dist.crx and dist.pem. The extension id isgenerated fromtied to this private key file, keep it around so that it is reused and does not change.<your-extension-id>
with the extension id fromchrome:https://extensions
.If someone uploads this to the chrome webstore and keeps the private key file safe the extension id would stay the same, there would be no need for this dance and we could just have one ff2mpv-chrome.json manifest.
NOTE: This has nothing to do with the existing chrome port by @DanSM-5 https://github.com/DanSM-5/ff2mpv https://chrome.google.com/webstore/detail/ff2mpv/ephjcajbkgplkjmelpglennepbpmdpjg . In the future maybe he could upload this version of the chrome port to the chrome webstore, instead of using a fork.