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

Update Manifest to V3, include mixed-mode tudor crown icons #186

Merged
merged 8 commits into from
Feb 19, 2024

Conversation

KludgeKML
Copy link
Contributor

@KludgeKML KludgeKML commented Feb 15, 2024

Compromise release version to cope with the fact that we need to update the crown logo and V2 manifests can no longer be deployed to Chrome Web Store.

  • Update Manifest to V3
  • Service worker replaces old events code
  • Chrome API calls updated to new versions
  • A/B testing support is temporarily removed from this version (to be re-added in a later version)

https://trello.com/c/liGfGsga/2174-migrate-to-a-service-worker

@KludgeKML KludgeKML force-pushed the update-manifest-to-v3-kml branch from 41f7a2b to ae88148 Compare February 19, 2024 11:35
KludgeKML and others added 7 commits February 19, 2024 11:36
Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
- New Chrome APIs mean we can't detect dark mode in a
  service worker, so remove dark mode versions and add
  mixed-mode-safe icons of the Tudor crown

Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
- A/B testing component temporarily disabled to
  allow deploy

Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
- Temporarily disable A/B testing related code
- chrome.tabe.executeScript -> chrome.scripting.executeScript
  (calling functions now have to be async)
- Remove reliance on component states that might not be available
  in async code
- explicit location parts in re-render popup code

Co-authored-by: Kashif Atcha <[email protected]>
Co-authored-by: Jon Kirwan <[email protected]>
Co-authored-by: Keith Lawrence <[email protected]>
@KludgeKML KludgeKML force-pushed the update-manifest-to-v3-kml branch from ae88148 to 1cbfa0a Compare February 19, 2024 11:38
@KludgeKML KludgeKML changed the title Update manifest to v3 (Keith's branch of the WIP) Update Manifest to V3, include mixed-mode tudor crown icons Feb 19, 2024
@KludgeKML KludgeKML marked this pull request as ready for review February 19, 2024 12:14
Copy link
Contributor

Choose a reason for hiding this comment

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

NIce work removing this!

Copy link
Contributor

@MartinJJones MartinJJones left a comment

Choose a reason for hiding this comment

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

Nice work on this update!

I've checked the code and referred to the Migration guide and all looks good to me - https://developer.chrome.com/docs/extensions/develop/migrate

I've also tested the changes using the unpacked chrome extensions in my browser and all the functionality still works as expected, knowing that the functionality for AB tests will be added back in the future.

@KludgeKML KludgeKML mentioned this pull request Feb 19, 2024
Closed
@KludgeKML KludgeKML merged commit d1261f0 into main Feb 19, 2024
1 check passed
@KludgeKML KludgeKML deleted the update-manifest-to-v3-kml branch February 19, 2024 16:20
@hannako
Copy link

hannako commented May 7, 2024

@KludgeKML are there any plans to add the ab test toggle back into the chrome extension?

yndajas added a commit that referenced this pull request Oct 15, 2024
In its current state, and presumably since the [Manifest V3
migration](#186),
the extension doesn't work on Firefox. However, it can be made fully
functional with a small change

When loading the extension in Firefox, the following is reported

```
There was an error during the temporary add-on installation.

Error details

background.service_worker is currently disabled
```

Firefox doesn't currently support `background.service_worker` in
Manifest V3: https://bugzilla.mozilla.org/show_bug.cgi?id=1573659. This
property was added when migrating to V3:
#186

If you remove this section from the manifest, it will work on V3. The
only caveat is that the icon won't change when on a GOV.UK page

I also tried changing `service_worker` to `scripts` and using an array
per [this
suggestion](mozilla/web-ext#2532 (comment)),
which also makes the extension functional, but doesn't make the
icon-changing script work. I suspect the `icon.js` code would need
updating for cross-browser support. Other suggestions later in that
thread might be worth exploring, but it might make more sense to hold
off until Firefox adds proper support if the user base is low
ChrisBAshton pushed a commit that referenced this pull request Oct 17, 2024
In its current state, and presumably since the [Manifest V3
migration](#186),
the extension doesn't work on Firefox. However, it can be made fully
functional with a small change

When loading the extension in Firefox, the following is reported

```
There was an error during the temporary add-on installation.

Error details

background.service_worker is currently disabled
```

Firefox doesn't currently support `background.service_worker` in
Manifest V3: https://bugzilla.mozilla.org/show_bug.cgi?id=1573659. This
property was added when migrating to V3:
#186

If you remove this section from the manifest, it will work on V3. The
only caveat is that the icon won't change when on a GOV.UK page

I also tried changing `service_worker` to `scripts` and using an array
per [this

suggestion](mozilla/web-ext#2532 (comment)),
which also makes the extension functional, but doesn't make the
icon-changing script work. I suspect the `icon.js` code would need
updating for cross-browser support. Other suggestions later in that
thread might be worth exploring, but it might make more sense to hold
off until Firefox adds proper support if the user base is low
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.

3 participants