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

Only inject hook.js script for Firefox. Use ExecutionWorld.MAIN for Chrome #1232

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

jerelmiller
Copy link
Member

@jerelmiller jerelmiller commented Feb 21, 2024

Fixes #1162

This is a revival of #1164 with some tweaks. Now that we are running Chrome on Manifest v3, we are able to use hook.js as a content script using ExecutionWorld.MAIN. Unfortunately this is not yet supported in Firefox, so we have to rely on script injection for this to work. The script injection used in #1164 is now added back, but only when building for Firefox.

@jerelmiller jerelmiller requested a review from a team as a code owner February 21, 2024 04:29
@phryneas
Copy link
Member

phryneas commented Feb 21, 2024

This looks good to me (tested locally in Chrome and Firefox), but I'm slightly concerned: didn't you see the extension break with the current Firefox code, in Firefox?

Could you reproduce that?

Base automatically changed from update-message-passing to main February 21, 2024 17:29
@jerelmiller
Copy link
Member Author

Talked in our 1:1. Posting the answer here for transparency:

I feel a bit more confident now in this change for a couple reasons:

@jerelmiller jerelmiller reopened this Feb 21, 2024
@jerelmiller
Copy link
Member Author

We are going to wait to merge this for a few days after 4.7.0 goes out to ensure we catch any bugs or new issues that arise from that release.

@jerelmiller
Copy link
Member Author

Putting in draft for now to avoid accidental merge until we are ready.

@jerelmiller jerelmiller marked this pull request as draft February 21, 2024 17:49
@jerelmiller jerelmiller marked this pull request as ready for review February 26, 2024 22:22
@jerelmiller
Copy link
Member Author

So far we've seen no reports that the latest update caused any more issues. I'll go ahead and merge this and get a release out tomorrow.

@jerelmiller jerelmiller merged commit 8ce6fef into main Feb 26, 2024
6 checks passed
@jerelmiller jerelmiller deleted the worlds branch February 26, 2024 22:25
@github-actions github-actions bot mentioned this pull request Feb 26, 2024
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.

Extension prevents websites (tested on atlassian.com) from loading styles
2 participants