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

Make compatible with Manifest v3 #256

Merged
merged 18 commits into from
May 7, 2023
Merged

Make compatible with Manifest v3 #256

merged 18 commits into from
May 7, 2023

Conversation

rafaelgomesxyz
Copy link
Member

@rafaelgomesxyz rafaelgomesxyz commented Jan 15, 2023

Fixes #156

NOTE: This should be carefully tested, as some things might still be broken. I'll continue to test on my end throughout this week.

This follows Chrome's migration guide and:

  • changes manifest version to v3
  • changes background script to service worker
  • separates permissions from host permissions
  • removes content security policy
  • renames browser action to action
  • removes web accessible resources
  • uses scripting API to inject content scripts
  • uses scripting API to inject functions (all functions that need to be injected now have to be added to Shared.functionsToInject on first run, because they need to be accessible from the service worker)
  • removes ports, as they're not compatible with service workers (the new method to identify when a content script has connected is to send a message to the service worker, and the new method to identify when a content script has disconnected is to add a listener to browser.tabs.onRemoved)
  • removes axios, as XMLHttpRequest isn't compatible with service workers (rate limiting has been removed as well, but our requests are already sequential, so they shouldn't cause many issues)
  • removes window/document references from background logic, as they're not available in service workers

There might be other changes that need to be made.

These changes should be fully backwards compatible with manifest v2. Webpack is configured to build v2 for Firefox and v3 for Chrome, as v3 isn't accepted in the Firefox store yet, as far as I could tell.

@rafaelgomesxyz rafaelgomesxyz added the enhancement New feature or request label Jan 15, 2023
@rafaelgomesxyz rafaelgomesxyz added this to the v1.0.0 milestone Jan 15, 2023
@rafaelgomesxyz rafaelgomesxyz self-assigned this Jan 15, 2023
@rafaelgomesxyz
Copy link
Member Author

@MrMamen It's a WIP, but if you already want to take a look.

@rafaelgomesxyz
Copy link
Member Author

@MrMamen Can we try to land this? I didn't test too much (also mostly just Netflix), but we can always refine it later. I'd like to release it on Chrome soon.

@MrMamen
Copy link
Member

MrMamen commented Mar 7, 2023

Yes. I'll test it a bit.

@MrMamen
Copy link
Member

MrMamen commented Mar 8, 2023

NRK doesn't work with scrobbeling. This is due to the injection script not working. (Only tested on Chrome).

@rafaelgomesxyz
Copy link
Member Author

Got it, thanks, I'll see if I can fix it.

@rafaelgomesxyz
Copy link
Member Author

@MrMamen Script injection for functions should be fixed.

@MrMamen
Copy link
Member

MrMamen commented Apr 15, 2023

@MrMamen Script injection for functions should be fixed.

It seems to me that the scrobbling part needed to be disabled and re-enabled after switching to this branch. (for NRK at least.) If there is no way to prevent this, perhaps the migrate script should simply disable the scrobbling for injection-script sites?

@rafaelgomesxyz
Copy link
Member Author

@MrMamen Script injection for functions should be fixed.

It seems to me that the scrobbling part needed to be disabled and re-enabled after switching to this branch. (for NRK at least.) If there is no way to prevent this, perhaps the migrate script should simply disable the scrobbling for injection-script sites?

But that's for Chrome, right? I don't think it will be possible to migrate from v2 to v3 on Chrome, because they have different IDs, so they should be two separate extensions when installed. That wasn't the case for you because you probably don't have the ID configured in your .env. Unfortunately, I think Chrome users will have to reconfigure their options.

@rafaelgomesxyz
Copy link
Member Author

I'll see if I can add an import/export functionality to allow users to move their data.

@MrMamen
Copy link
Member

MrMamen commented Apr 16, 2023

I'll see if I can add an import/export functionality to allow users to move their data.

Om not sure how important this is. More important to get this to shipped. We should be more careful with these things after it hits the Web Stores.

@rafaelgomesxyz
Copy link
Member Author

Ok, I've actually already uploaded it to the Chrome store because I wanted to test something: https://chrome.google.com/webstore/detail/universal-trakt-scrobbler/mbhadeogepkjdjeikcckdkjdjhhkhlid

But before we add it to the README I want to land another PR that I'll open later today that allows users to do a full initial sync.

@MrMamen
Copy link
Member

MrMamen commented Apr 19, 2023

I experience that at least netflix history keeps loading/spinning forever, with no obvious error..

Copy link
Member

@MrMamen MrMamen left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@rafaelgomesxyz rafaelgomesxyz merged commit 397c195 into master May 7, 2023
@rafaelgomesxyz rafaelgomesxyz deleted the manifest-v3 branch May 7, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to manifest v3
2 participants