Content script context event #1778
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR removes the script lifecycle management functionality from the ContentScriptContext class and adds a guard condition to prevent event listeners when the context is invalid. The changes eliminate the automatic invalidation mechanism that was used to stop older content script instances when new ones were loaded.
- Removes script lifecycle management (starting/stopping old scripts detection)
- Simplifies the constructor by removing frame detection and script lifecycle setup
- Adds a guard condition in addEventListener to prevent registration when context is invalid
|
|
||
| target.addEventListener?.( | ||
| eventType, | ||
| () => this.isInvalid, // signals the abort controller when invalid |
There was a problem hiding this comment.
The first addEventListener call is registering a function that returns a boolean as an event handler, which is incorrect. Event handlers should be functions that handle events, not functions that return boolean values.
| () => this.isInvalid, // signals the abort controller when invalid | |
| (event) => { | |
| if (this.isInvalid) { | |
| this.abortController.abort(); // Abort the controller if invalid | |
| } | |
| }, |
Copilot uses AI. Check for mistakes.
| const eventType = type.startsWith('wxt:') ? getUniqueEventName(type) : type | ||
|
|
||
| target.addEventListener?.( | ||
| eventType, | ||
| () => this.isInvalid, // signals the abort controller when invalid | ||
| { | ||
| capture: true, | ||
| signal: this.signal, | ||
| }, | ||
| ); | ||
|
|
||
| target.addEventListener?.( | ||
| type.startsWith('wxt:') ? getUniqueEventName(type) : type, | ||
| eventType, | ||
| handler, | ||
| { | ||
| ...options, |
There was a problem hiding this comment.
The eventType variable is calculated but then the same logic is duplicated by having two separate addEventListener calls. This creates unnecessary complexity and potential for inconsistency.
Copilot uses AI. Check for mistakes.
|
Oops did not want to assign copilot to this, could you add more context to this (haha funny coincidence) to better understand this? |
Yea I don't care if this PR gets selected, but the problem that we're facing is this. Every content script injection will cause this class to initiate a This PR is a different approach, maybe not the correct one, but basically defers removal of these event handlers to the next time the events fire. This means it's not immediate, but there's no window messaging involved and it solves the former issues I described. If this isn't something we want to change or do, that's totally fair, but then at a minimum, we should have some way in wxt to prevent this |
Using One alternative would be using a custom event based on the extension ID on the window object. Websites would at least need to check for every extension by their ID separately to fingerprint users. |
Overview
Manual Testing
Related Issue
This PR closes #<issue_number>