Skip to content

fix: Warned about missing extension IDs when running Firefox - #3879

Open
Nikshay1 wants to merge 2 commits into
mozilla:masterfrom
Nikshay1:fix/3547-warn-missing-firefox-id
Open

Nikshay1 wants to merge 2 commits into
mozilla:masterfrom
Nikshay1:fix/3547-warn-missing-firefox-id

Conversation

@Nikshay1

@Nikshay1 Nikshay1 commented Oct 2, 2026

Copy link
Copy Markdown

Fixes #3547

web-ext run can temporarily load a Firefox extension without an explicit ID, which can make it surprising when the same manifest fails during permanent installation or signing. This change prints one actionable warning before browser startup when a Firefox runner is selected and the manifest has no explicit extension ID. Temporary loading still proceeds.

The check reuses getManifestId() to respect browser_specific_settings.gecko precedence and support legacy applications.gecko.id. It covers the default target, Firefox Desktop, Firefox Android, and mixed targets, without warning for Chromium-only runs or printing twice when both Firefox targets are selected. The message says permanent installation or signing may require an ID, since Manifest V2 signing can generate one.

Unit tests cover target combinations, Manifest V2 and V3, modern and legacy IDs, empty IDs, manifest precedence, warning timing, and unchanged validation failures. Two existing parser stubs now return valid manifest fixtures. A CLI regression test uses the repository's fake Firefox binary to verify that the warning reaches stderr without --verbose, appears once, and still permits successful temporary installation.

Validation:

  • npm run build passed.
  • npm test passed: 603 tests, no lint errors (one existing unused-disable warning in index.js).
  • npm run test-functional passed: 21 tests, 4 pending (real-Chrome and production import tests).
  • Prettier checks passed for all three changed files; npm run prettier-dev also passed.

@Nikshay1

Copy link
Copy Markdown
Author

I pushed a second commit, 20aa29c, to fix the unit test failures on Node 22 (build-nodejs-current and test-windows).

The 11 tests that expect the warning failed there with expected [] to have a length of 1. The cause was in the test setup, not in run.js. mockModule did not wait for td.replaceEsm, so the logger module that the tests stubbed could be an older copy than the one run.js imported. The warning was logged, but through a consoleStream that was not stubbed. On Node 24 the timing happened to hide this.

The change:

  • mockModule in tests/unit/helpers.js now returns the td.replaceEsm promise.
  • The beforeEach in tests/unit/test-cmd/test.run.js awaits its three mockModule calls.

tests/unit/test.web-ext.js also calls mockModule without await. I left it as it is to keep this PR small, but I can add it here if you prefer.

CI on 20aa29c: the unit tests now pass in all four jobs. Two jobs are still red, and I believe neither comes from this PR:

  • build-nodejs-current fails in the npm run audit-deps step, which also fails on master.
  • test-windows timed out in the functional tests, in web-ext run -t chromium > with auto-reload > simulate Chrome 137 (tests/functional/test.cli.run-target-chromium.js). This PR does not change that file, and the same suite timed out on master in job 21498. Could someone re-run test-windows?

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.

Warn about missing browser_specific_settings.gecko.id in the manifest.json on web-ext run commands targeting Firefox

1 participant