fix: Scripts injection for content-scripts on Firefox MV2 target

[PatrykKuniczak]

Overview

I've fixed issue with awaited injectScript for Firefox MV2, which isn't included in build files.

Manual Testing

Let's try code from mentioned issue reproduction

This PR closes #2183

Code with example for that'll be merged in #2233

[netlify]

✅ Deploy Preview for creative-fairy-df92c4 ready!

Name	Link
🔨 Latest commit	fd23425
🔍 Latest deploy log	https://app.netlify.com/projects/creative-fairy-df92c4/deploys/69dcf3271e72b70008e27d84
😎 Deploy Preview	https://deploy-preview-2232--creative-fairy-df92c4.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.50%. Comparing base (2a21279) to head (fd23425).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/wxt/src/utils/inject-script.ts	0.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2232      +/-   ##
==========================================
+ Coverage   76.66%   79.50%   +2.83%     
==========================================
  Files         117      130      +13     
  Lines        3163     3805     +642     
  Branches      711      860     +149     
==========================================
+ Hits         2425     3025     +600     
- Misses        656      694      +38     
- Partials       82       86       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pkg-pr-new]

Open in StackBlitz

@wxt-dev/analytics

npm i https://pkg.pr.new/@wxt-dev/analytics@2232

@wxt-dev/auto-icons

npm i https://pkg.pr.new/@wxt-dev/auto-icons@2232

@wxt-dev/browser

npm i https://pkg.pr.new/@wxt-dev/browser@2232

@wxt-dev/i18n

npm i https://pkg.pr.new/@wxt-dev/i18n@2232

@wxt-dev/is-background

npm i https://pkg.pr.new/@wxt-dev/is-background@2232

@wxt-dev/module-react

npm i https://pkg.pr.new/@wxt-dev/module-react@2232

@wxt-dev/module-solid

npm i https://pkg.pr.new/@wxt-dev/module-solid@2232

@wxt-dev/module-svelte

npm i https://pkg.pr.new/@wxt-dev/module-svelte@2232

@wxt-dev/module-vue

npm i https://pkg.pr.new/@wxt-dev/module-vue@2232

@wxt-dev/runner

npm i https://pkg.pr.new/@wxt-dev/runner@2232

@wxt-dev/storage

npm i https://pkg.pr.new/@wxt-dev/storage@2232

@wxt-dev/unocss

npm i https://pkg.pr.new/@wxt-dev/unocss@2232

@wxt-dev/webextension-polyfill

npm i https://pkg.pr.new/@wxt-dev/webextension-polyfill@2232

wxt

npm i https://pkg.pr.new/wxt@2232

commit: fd23425

[nines75]

@PatrykKuniczak My explanation at #2183 (comment) was insufficient. With the current code, injectScript will never resolve in MV3.
What I meant was to separate the creation of the Promise via makeLoadedPromise from the timing of using await on it, just like the injectScript code prior to this PR.

Current code in the PR:

if (isManifestV3) {
  // Event handlers (load, error) are registered before the script execution.
  // However, since `await` is used here, it will wait forever for the `load` event to fire.
  await makeLoadedPromise(script);
}

await options?.modifyScript?.(script);

(document.head ?? document.documentElement).append(script);

if (!options?.keepInDom) {
  script.remove();
}

Previous code (current main branch):

// Event handlers are registered before the script execution.
// Since `await` is not used, execution doesn't pause here.
const loadedPromise = makeLoadedPromise(script);

await options?.modifyScript?.(script);

// At this point, the event handlers are guaranteed to be registered.
(document.head ?? document.documentElement).append(script);

if (!options?.keepInDom) {
  script.remove();
}

// Wait for the `load` event to fire here.
// However, the `load` event doesn't fire in MV2.
await loadedPromise;

Based on the above, I think it would be better to implement the fix as follows:

// Register the event handlers only for MV3.
const loadedPromise = isManifestV3 ? makeLoadedPromise(script) : undefined;

await options?.modifyScript?.(script);

(document.head ?? document.documentElement).append(script);

if (!options?.keepInDom) {
  script.remove();
}

if (isManifestV3) {
  await loadedPromise;
}
// This alternative might be better:
// if (loadedPromise) {
//   await loadedPromise;
// }

I'm not entirely sure if this code is perfectly correct, but I have confirmed that it works properly on Firefox MV2 and Chrome MV3.

[PatrykKuniczak]

@nines75 I was wondering, if this is necessary to do it that way, because it work's anyway.

But i've checked it, and

const loadedPromise = isManifestV3 ? makeLoadedPromise(script) : Promise.resolve();

works as well, we need to wait for @aklinker1 review, if we'll approve your suggestion, i'll do it, that's no problem :)

I'm not sure, my simplification is good approach 😆

[aklinker1]

Confirmed its working with the latest changes.

[PatrykKuniczak]

@aklinker1 Yeah, that one also works.
I've previously Promise.resolve() on the undefined place, but i don't know why it wasn't work.
But it's good right now and let's leave it what it is 😄