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

fix(background): use chrome.scripting.ExecutionWorld.MAIN for firefox compatibility #404

Merged
merged 3 commits into from
Oct 31, 2024

Conversation

ttys0dev
Copy link
Contributor

@ttys0dev ttys0dev commented Oct 4, 2024

It seems firefox is picky here and won't accept the a MAIN string.

@ttys0dev ttys0dev force-pushed the fix-firefox-world branch 2 times, most recently from c4eef14 to d197f9c Compare October 4, 2024 01:41
@ttys0dev
Copy link
Contributor Author

ttys0dev commented Oct 4, 2024

rebased

@ttys0dev
Copy link
Contributor Author

ttys0dev commented Oct 4, 2024

@ERosendo Does this look good to merge?

@mlissner
Copy link
Member

mlissner commented Oct 7, 2024

@ttys0dev, what's the impact of this being wrong?

@ttys0dev
Copy link
Contributor Author

ttys0dev commented Oct 7, 2024

what's the impact of this being wrong?

I was seeing an exception being thrown.

@mlissner
Copy link
Member

mlissner commented Oct 7, 2024

What version of FF?

@ttys0dev
Copy link
Contributor Author

ttys0dev commented Oct 7, 2024

What version of FF?

Was some ESR version I think where I saw the error, may have updated it since I tested though(currently on version 128 looks like).

@mlissner
Copy link
Member

mlissner commented Oct 8, 2024

Cool. We're not making much effort to support old FF versions, but if this works on both, we might as well merge. I leave it to Eduardo to make that decision.

@ERosendo
Copy link
Contributor

@ttys0dev I've tested this change in the latest versions of Chrome, Firefox, and Safari. While it works properly in Chrome and Firefox, it breaks the extension in Safari. I've verified that the code from the main branch works as expected in Safari.

- Detect Safari browser explicitly
- Adjust execution world to 'MAIN' for Safari, 'chrome.scripting.ExecutionWorld.MAIN' for other browsers
@ERosendo ERosendo merged commit 7d87622 into freelawproject:main Oct 31, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants