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

Add support for Chromium based browser #14

Merged
merged 1 commit into from
Aug 18, 2024

Conversation

samyakbardiya
Copy link
Contributor

Added support for Chromium-based browsers. I tested the code in both Chrome 129.0.6643.2 dev (arm64) & Firefox 130.0b3 (64-bit), and it works perfectly.

Changes

  • Using chrome.* instead of browser.* for only Chromium-based browsers, as requested to use browser.* for the added advantages in Firefox.
  • Added both background.service_worker & background.scripts to manifest.json. As mentioned here, this works, but it results in warnings in both Firefox and Chrome about the other value being unrecognized.
  • Removed loadReplace as there is no counterpart in the Chromium side.

@dmlls
Copy link
Owner

dmlls commented Aug 18, 2024

Hi @samyakbardiya and thank you for your contribution!

Apparently from Firefox 121, background.service_worker can coexist with background.scripts on Firefox (the former will just be ignored), which indeed allows us to include both in the manifest.

I'll merge this PR, solve the conflicts from the Manifest v3 migration PR, and publish a new release.

@dmlls dmlls merged commit fc7af1b into dmlls:manifest-v3 Aug 18, 2024
1 check failed
@samyakbardiya
Copy link
Contributor Author

Thanks, @dmlls :)

Also, I have been using this extensively in Arc (Chromium base) browser since the PR and have found only one issue so far. The issue is that many times, the bang conversion does not happen the very first time. However, if I simply refresh, it then happens. I haven't been able to understand why this is occurring.

@dmlls
Copy link
Owner

dmlls commented Aug 22, 2024

Thanks for the extensive testing!

What do you exactly mean by conversion? The bang doesn't trigger?

Maybe you can try to load the add-on in developer mode and check if the bangs are loaded correctly in the first place. I'm already working on the release, I will also of course test things thoroughly before releasing.

@samyakbardiya
Copy link
Contributor Author

Yes, I meant the triggering. And yes, I'm using the add-on in develop mode only, and it has been loaded properly, though, with 2 warnings about the manifest.

Warnings:

  • 'background.scripts' requires manifest version of 2 or lower.
  • Unrecognized manifest key 'browser_specific_settings'.

@dmlls
Copy link
Owner

dmlls commented Aug 23, 2024

I see. Maybe you can try to add a console.log(bangs) in this line to check whether the bangs object gets populated correctly?

@samyakbardiya
Copy link
Contributor Author

Sure @dmlls, will check it for a few days.

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.

2 participants