fix: web-ext should look for the extension id in browser_specific_settings and fallback to applications

.flowconfig

@@ -4,6 +4,7 @@
 
 [options]
 module.system=node
+esproposal.optional_chaining=enable
 log.file=./artifacts/flow.log
 
 [ignore]

src/util/manifest.js

@@ -15,7 +15,7 @@ const log = createLogger(__filename);
 // getValidatedManifest helper types and implementation
 
 export type ExtensionManifestApplications = {|
-  gecko: {|
+  gecko?: {|
     id?: string,
     strict_min_version?: string,
     strict_max_version?: string,
@@ -28,6 +28,7 @@ export type ExtensionManifest = {|
   version: string,
   default_locale?: string,
   applications?: ExtensionManifestApplications,
+  browser_specific_settings?: ExtensionManifestApplications,
   permissions?: Array<string>,
 |};
 
@@ -85,6 +86,19 @@ export default async function getValidatedManifest(
 
 
 export function getManifestId(manifestData: ExtensionManifest): string | void {
-  return manifestData.applications ?
-    manifestData.applications.gecko.id : undefined;
+  const manifestApps = [
+    manifestData.browser_specific_settings,
+    manifestData.applications,
+  ];
+  for (const apps of manifestApps) {
+    // If both bss and applicants contains a defined gecko property,
+    // we prefer bss even if the id property isn't available.
+    // This match what Firefox does in this particular scenario, see
+    // https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/toolkit/mozapps/extensions/internal/XPIInstall.jsm#470-474,488
+    if (apps?.gecko) {
+      return apps.gecko.id;
+    }
+  }
+
+  return undefined;
 }

tests/unit/test-cmd/test.sign.js

@@ -279,7 +279,7 @@ describe('sign', () => {
             apiSecret: stubs.signingConfig.apiSecret,
             apiUrlPrefix: stubs.signingConfig.apiUrlPrefix,
             downloadDir: artifactsDir,
-            id: applications.gecko.id,
+            id: applications.gecko?.id,
             timeout: stubs.signingConfig.timeout,
             version: stubs.preValidatedManifest.version,
             xpiPath: stubs.buildResult.extensionPath,

tests/unit/test-extension-runners/test.firefox-desktop.js

@@ -284,8 +284,7 @@ describe('util/extension-runners/firefox-desktop', () => {
 
     assert.equal(install.asProxy, true);
     assert.equal(install.manifestData.applications.gecko.id,
-                 manifestData.applications &&
-                 manifestData.applications.gecko.id);
+                 manifestData.applications?.gecko?.id);
     assert.deepEqual(install.profile, fakeProfile);
     // This needs to be the source of the extension.
     assert.equal(install.extensionPath, sourceDir);

tests/unit/test-util/test.manifest.js

@@ -211,14 +211,68 @@ describe('util/manifest', () => {
   });
 
   describe('getManifestId', () => {
+    const id = 'basic-manifest@web-ext-test-suite';
+
+    ['applications', 'browser_specific_settings'].forEach((key: string) => {
+
+      describe(`with ${key}`, () => {
+
+        it('returns gecko.id if present', () => {
+          assert.equal(getManifestId({
+            ...manifestWithoutApps,
+            [key]: basicManifest.applications,
+          }), id);
+        });
+
+        it('returns undefined when gecko does not exist', () => {
+          assert.equal(
+            getManifestId({
+              ...manifestWithoutApps,
+              [key]: {},
+            }),
+            undefined
+          );
+        });
+
+      });
 
-    it('returns a gecko ID', () => {
-      assert.equal(getManifestId(basicManifest),
-                   'basic-manifest@web-ext-test-suite');
     });
 
-    it('returns undefined when ID is not specified', () => {
-      assert.strictEqual(getManifestId(manifestWithoutApps), undefined);
+    describe('with both applications and browser_specific_settings', () => {
+      const bssId = 'id@from-bss-prop';
+      const appId = 'id@from-app-prop';
+
+      it('does prefer bss if it includes a gecko object', () => {
+        assert.equal(getManifestId({
+          ...manifestWithoutApps,
+          browser_specific_settings: {gecko: {id: bssId}},
+          applications: {gecko: {id: appId}},
+        }), bssId);
+
+        // This test that we are matching what Firefox does in this scenario.
+        assert.equal(getManifestId({
+          ...manifestWithoutApps,
+          browser_specific_settings: {gecko: {}},
+          applications: {gecko: {id: appId}},
+        }), undefined);
+      });
+
+      it('does fallback to applications if bss.gecko is undefined', () => {
+        assert.equal(getManifestId({
+          ...manifestWithoutApps,
+          browser_specific_settings: {},
+          applications: {gecko: {id: appId}},
+        }), appId);
+      });
+
+    });
+
+    describe('without applications and browser_specific_settings', () => {
+
+      it('returns undefined when ID is not specified', () => {
+        assert.strictEqual(getManifestId(manifestWithoutApps), undefined);
+      });
+
     });
 
   });