chore: Tweaks to better match Firefox behavior and small fixes for recent flow typecheck errors

Author: rpl

.flowconfig

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

src/util/manifest.js

@@ -91,7 +91,11 @@ export function getManifestId(manifestData: ExtensionManifest): string | void {
     manifestData.applications,
   ];
   for (const apps of manifestApps) {
-    if (apps && apps.gecko && apps.gecko.id) {
+    // 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;
     }
   }

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 && 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,9 +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 &&
-                 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

@@ -213,7 +213,7 @@ describe('util/manifest', () => {
   describe('getManifestId', () => {
     const id = 'basic-manifest@web-ext-test-suite';
 
-    ['applications', 'browser_specific_settings'].forEach((key) => {
+    ['applications', 'browser_specific_settings'].forEach((key: string) => {
 
       describe(`with ${key}`, () => {
 
@@ -238,6 +238,35 @@ describe('util/manifest', () => {
 
     });
 
+    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', () => {