Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions packages/wxt/src/utils/content-script-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,19 @@ export class ContentScriptContext implements AbortController {
'wxt:content-script-started',
);

private isTopFrame = window.self === window.top;
private id: string;
private abortController: AbortController;
private locationWatcher = createLocationWatcher(this);
private receivedMessageIds = new Set<string>();

constructor(
private readonly contentScriptName: string,
public readonly options?: Omit<ContentScriptDefinition, 'main'>,
) {
this.id = Math.random().toString(36).slice(2);
this.abortController = new AbortController();

if (this.isTopFrame) {
this.listenForNewerScripts({ ignoreFirstEvent: true });
this.stopOldScripts();
} else {
this.listenForNewerScripts();
}
this.stopOldScripts();
this.listenForNewerScripts();
}

get signal() {
Expand Down Expand Up @@ -243,43 +239,56 @@ export class ContentScriptContext implements AbortController {
}

stopOldScripts() {
// Use postMessage so it get's sent to all the frames of the page.
Copy link
Contributor Author

@namukang namukang Oct 15, 2025

Choose a reason for hiding this comment

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

In case there's concern that changing from window.postMessage to document.dispatchEvent will change behavior when it comes to other frames: This comment is incorrect because window.postMessage only sends a message to the targeted window. It doesn't automatically send messages to other frames — you'd need to specifically target those windows in order to send a message.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the * target mean we're sending messages to all frames?

Copy link
Member

@aklinker1 aklinker1 Oct 27, 2025

Choose a reason for hiding this comment

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

This is my main concern. Basically, this class is meant to properly stop and shut down a content script when an extension updates. In production or in development, you will encounter bugs if we don't properly stop ALL the content scripts running on a page. So if in one update, a content script changes from allFrames: true to allFrames: false, some scripts running in iframes won't be stopped by using custom events.

Because I'm 99% sure targeting "*" sends a message to all iframes on the page, and any level.

Now, that said, the current approach has tradeoffs as well, this race condition you found. If we don't send a message to all frames, then technically those scripts will still stop after they try and use an extension API, only to find that they can't because the context was invalidated after the update.

On a separate note, another thing to consider is that this is a breaking change that will effect all WXT extensions on their next update. We're going to send a custom event, but the old version of the extension will be listening for messages. This could be resolved by continuing to send the message...

Can I get some more context around why you would inject the same script multiple times in quick succession, like from the reproduction? I need more info to decide how likely this race condition is to weigh it against merging this PR and changing the behavior.

Copy link
Contributor Author

@namukang namukang Oct 28, 2025

Choose a reason for hiding this comment

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

window.postMessage and iframes

Targeting "*" doesn't send a message to all iframes on the page. The targetOrigin parameter of window.postMessage specifies which origin is allowed to receive the message.

In order to send a message to an iframe, the iframe's window needs to be targeted directly: e.g. iframe.contentWindow.postMessage(message, targetOrigin);

From MDN:

Furthermore, an accessing script must have obtained the window object of the accessed document beforehand. This can occur through methods such as window.open() for popups or iframe.contentWindow for iframes.

The current code of window.postMessage(message, "*") simply sends a message to the current window and allows any origin to receive it, but it doesn't send a message to iframes.

That means the current approach actually has a bug where content scripts inside iframes are not invalidated at all when the content script is injected again (e.g. when the extension updates).

I've created a video demonstration of the behavior with allFrames: true with the current implementation and the fix proposed in this PR: https://www.loom.com/share/a172168ee3744979b949ef8411d126bc

Backwards compatibility

I've added to the PR to temporarily continue sending the message via window.postMessage as well in order to invalidate content scripts from before this change. This makes this change backwards compatible and not a breaking change.

For the particular scenario you gave: The new implementation doesn't invalidate content scripts inside iframes when injecting a content script is changed from allFrames: true to allFrames: false. But this wouldn't be breaking change since the current implementation doesn't handle that case either. I personally think this is an edge case that's not worth worrying about (at least in the scope of this PR), in the same way that WXT doesn't currently try to invalidate content scripts that were injected in previous versions of an extension but were removed or renamed in the current version. I think it'd be fine to let them stop when trying to use an extension API.

Overall, I think this change is worth making because it (1) fixes the bug with content scripts inside iframes not being invalidated and (2) entirely eliminates race conditions with invalidations given the synchronous nature of document.dispatchEvent.

Context for multiple injections

Here's why the content script is injected multiple times in quick succession in my extension (Easy Scraper):

When the user clicks the extension icon to open the popup window, a content script is injected into the current tab and is injected again whenever a new page is loaded in the current tab. The popup window does this by having a listener for browser.tabs.onUpdated and checking for changeInfo.status === "complete" to detect when a new page is loaded.

However, there are cases when a page causes browser.tabs.onUpdated to fire multiple times with changeInfo.status === "complete". For example, it's fired with status complete again when the page sets the hash (e.g. window.location.hash = "section") or changes the URL without reloading the page (e.g. window.history.pushState({}, '', '/new-url');.

So there are cases where the initial page load causes browser.tabs.onUpdated to fire in quick succession, which causes the popup window to inject the content script multiple times. (There's a check that runs to see if the current tab already has the content script before it tries to inject it, but in this case, that check fails because no content script has been injected yet when onUpdated is fired multiple times.)

For extensions that dynamically inject the content script using browser.tabs.onUpdated (as in my case where the activeTab permission is used along with a popup), this is a fairly unavoidable scenario.

document.dispatchEvent(
new CustomEvent(ContentScriptContext.SCRIPT_STARTED_MESSAGE_TYPE, {
detail: {
contentScriptName: this.contentScriptName,
messageId: this.id,
},
}),
);

// Send message using `window.postMessage` for backwards compatibility to invalidate old versions before WXT changed to `document.dispatchEvent`
// TODO: Remove this once WXT version using `document.dispatchEvent` has been released for a while
window.postMessage(
{
type: ContentScriptContext.SCRIPT_STARTED_MESSAGE_TYPE,
contentScriptName: this.contentScriptName,
messageId: Math.random().toString(36).slice(2),
messageId: this.id,
},
'*',
);
}

verifyScriptStartedEvent(event: MessageEvent) {
const isScriptStartedEvent =
event.data?.type === ContentScriptContext.SCRIPT_STARTED_MESSAGE_TYPE;
verifyScriptStartedEvent(event: CustomEvent) {
const isSameContentScript =
event.data?.contentScriptName === this.contentScriptName;
const isNotDuplicate = !this.receivedMessageIds.has(event.data?.messageId);
return isScriptStartedEvent && isSameContentScript && isNotDuplicate;
event.detail?.contentScriptName === this.contentScriptName;
// Handle case where website dispatches the event again for some reason
const isFromSelf = event.detail?.messageId === this.id;
return isSameContentScript && !isFromSelf;
}

listenForNewerScripts(options?: { ignoreFirstEvent?: boolean }) {
let isFirst = true;

const cb = (event: MessageEvent) => {
if (this.verifyScriptStartedEvent(event)) {
this.receivedMessageIds.add(event.data.messageId);

const wasFirst = isFirst;
isFirst = false;
if (wasFirst && options?.ignoreFirstEvent) return;

this.notifyInvalidated();
listenForNewerScripts() {
const cb: EventListener = (event) => {
if (
!(event instanceof CustomEvent) ||
!this.verifyScriptStartedEvent(event)
) {
return;
}
this.notifyInvalidated();
};

addEventListener('message', cb);
this.onInvalidated(() => removeEventListener('message', cb));
document.addEventListener(
ContentScriptContext.SCRIPT_STARTED_MESSAGE_TYPE,
cb,
);
this.onInvalidated(() =>
document.removeEventListener(
ContentScriptContext.SCRIPT_STARTED_MESSAGE_TYPE,
cb,
),
);
}
}

Expand Down