[bug] Patch the windows menu bar regressions (#1273)

* [bug] Patch the windows menu bar regressions

* [chore] Update jslib

jslib

@@ -1 +1 @@
-Subproject commit ca5b057b43ddf1ad671385b589395a91acd808b6
+Subproject commit e372bf242b24f55c1142e33693ad2c801ab74c93

src/main/menu.about.ts

@@ -3,7 +3,7 @@ import { BrowserWindow, clipboard, dialog, MenuItemConstructorOptions } from "el
 import { I18nService } from "jslib-common/abstractions/i18n.service";
 
 import { UpdaterMain } from "jslib-electron/updater.main";
-import { isMac, isSnapStore, isWindowsStore } from "jslib-electron/utils";
+import { isSnapStore, isWindowsStore } from "jslib-electron/utils";
 
 import { IMenubarMenu } from "./menubar";
 

src/main/menu.bitwarden.ts

@@ -1,17 +1,18 @@
-import { BrowserWindow, dialog, MenuItem, MenuItemConstructorOptions } from "electron";
+import { BrowserWindow, MenuItemConstructorOptions } from "electron";
 
 import { I18nService } from "jslib-common/abstractions/i18n.service";
 import { MessagingService } from "jslib-common/abstractions/messaging.service";
 
 import { UpdaterMain } from "jslib-electron/updater.main";
-import { isMacAppStore, isSnapStore, isWindowsStore } from "jslib-electron/utils";
+import { isMac } from "jslib-electron/utils";
 
 import { IMenubarMenu } from "./menubar";
 
+import { FirstMenu } from "./menu.first";
 import { MenuAccount } from "./menu.updater";
 
 // AKA: "FirstMenu" or "MacMenu" - the first menu that shows on all macOs apps
-export class BitwardenMenu implements IMenubarMenu {
+export class BitwardenMenu extends FirstMenu implements IMenubarMenu {
   readonly id: string = "bitwarden";
   readonly label: string = "Bitwarden";
 
@@ -24,6 +25,7 @@ export class BitwardenMenu implements IMenubarMenu {
     items.push(this.lock);
     items.push(this.lockAll);
     items.push(this.logOut);
+    items.push(this.separator);
     items.push(this.services);
 
     if (
@@ -45,13 +47,6 @@ export class BitwardenMenu implements IMenubarMenu {
     return items;
   }
 
-  private readonly _i18nService: I18nService;
-  private readonly _updater: UpdaterMain;
-  private readonly _messagingService: MessagingService;
-  private readonly _accounts: { [userId: string]: MenuAccount };
-  private readonly _window: BrowserWindow;
-  private readonly _isLocked: boolean;
-
   constructor(
     i18nService: I18nService,
     messagingService: MessagingService,
@@ -60,16 +55,7 @@ export class BitwardenMenu implements IMenubarMenu {
     accounts: { [userId: string]: MenuAccount },
     isLocked: boolean
   ) {
-    this._i18nService = i18nService;
+    super(i18nService, messagingService, updater, window, accounts, isLocked);
-    this._updater = updater;
-    this._messagingService = messagingService;
-    this._window = window;
-    this._accounts = accounts;
-    this._isLocked = isLocked;
-  }
-
-  private get hasAccounts(): boolean {
-    return this._accounts != null && Object.keys(this._accounts).length > 0;
   }
 
   private get aboutBitwarden(): MenuItemConstructorOptions {
@@ -77,118 +63,17 @@ export class BitwardenMenu implements IMenubarMenu {
       id: "aboutBitwarden",
       label: this.localize("aboutBitwarden"),
       role: "about",
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
 
-  private get checkForUpdates(): MenuItemConstructorOptions {
-    return {
-      id: "checkForUpdates",
-      label: this.localize("checkForUpdates"),
-      click: (menuItem) => this.checkForUpdate(menuItem),
-      visible: !isMacAppStore() && !isWindowsStore() && !isSnapStore(),
-    };
-  }
-
-  private get separator(): MenuItemConstructorOptions {
-    return {
-      type: "separator",
-    };
-  }
-
-  private get settings(): MenuItemConstructorOptions {
-    return {
-      id: "settings",
-      label: this.localize(process.platform === "darwin" ? "preferences" : "settings"),
-      click: () => this.sendMessage("openSettings"),
-      accelerator: "CmdOrCtrl+,",
-      enabled: !this._isLocked,
-    };
-  }
-
-  private get lock(): MenuItemConstructorOptions {
-    return {
-      id: "lock",
-      label: this.localize("lockVault"),
-      submenu: this.lockSubmenu,
-      enabled: this.hasAccounts,
-    };
-  }
-
-  private get lockSubmenu(): MenuItemConstructorOptions[] {
-    const value: MenuItemConstructorOptions[] = [];
-    for (const userId in this._accounts) {
-      if (userId == null) {
-        continue;
-      }
-
-      value.push({
-        label: this._accounts[userId].email,
-        id: `lockNow_${this._accounts[userId].userId}`,
-        click: () => this.sendMessage("lockVault", { userId: this._accounts[userId].userId }),
-        enabled: !this._accounts[userId].isLocked,
-        visible: this._accounts[userId].isAuthenticated,
-      });
-    }
-    return value;
-  }
-
-  private get lockAll(): MenuItemConstructorOptions {
-    return {
-      id: "lockAllNow",
-      label: this.localize("lockAllVaults"),
-      click: () => this.sendMessage("lockAllVaults"),
-      accelerator: "CmdOrCtrl+L",
-      enabled: this.hasAccounts,
-    };
-  }
-
-  private get logOut(): MenuItemConstructorOptions {
-    return {
-      id: "logOut",
-      label: this.localize("logOut"),
-      submenu: this.logOutSubmenu,
-      enabled: this.hasAccounts,
-    };
-  }
-
-  private get logOutSubmenu(): MenuItemConstructorOptions[] {
-    const value: MenuItemConstructorOptions[] = [];
-    for (const userId in this._accounts) {
-      if (userId == null) {
-        continue;
-      }
-
-      value.push({
-        label: this._accounts[userId].email,
-        id: `logOut_${this._accounts[userId].userId}`,
-        click: async () => {
-          const result = await dialog.showMessageBox(this._window, {
-            title: this.localize("logOut"),
-            message: this.localize("logOut"),
-            detail: this.localize("logOutConfirmation"),
-            buttons: [this.localize("logOut"), this.localize("cancel")],
-            cancelId: 1,
-            defaultId: 0,
-            noLink: true,
-          });
-          if (result.response === 0) {
-            this.sendMessage("logout", { userId: this._accounts[userId].userId });
-          }
-        },
-        visible: this._accounts[userId].isAuthenticated,
-      });
-    }
-    return value;
-  }
-
   private get services(): MenuItemConstructorOptions {
     return {
       id: "services",
       label: this.localize("services"),
       role: "services",
       submenu: [],
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
 
@@ -197,7 +82,7 @@ export class BitwardenMenu implements IMenubarMenu {
       id: "hideBitwarden",
       label: this.localize("hideBitwarden"),
       role: "hide",
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
 
@@ -206,7 +91,7 @@ export class BitwardenMenu implements IMenubarMenu {
       id: "hideOthers",
       label: this.localize("hideOthers"),
       role: "hideOthers",
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
 
@@ -215,7 +100,7 @@ export class BitwardenMenu implements IMenubarMenu {
       id: "showAll",
       label: this.localize("showAll"),
       role: "unhide",
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
 
@@ -224,21 +109,7 @@ export class BitwardenMenu implements IMenubarMenu {
       id: "quitBitwarden",
       label: this.localize("quitBitwarden"),
       role: "quit",
-      visible: isMacAppStore(),
+      visible: isMac(),
     };
   }
-
-  private localize(s: string) {
-    return this._i18nService.t(s);
-  }
-
-  private async checkForUpdate(menuItem: MenuItem) {
-    menuItem.enabled = false;
-    this._updater.checkForUpdate(true);
-    menuItem.enabled = true;
-  }
-
-  private sendMessage(message: string, args?: any) {
-    this._messagingService.send(message, args);
-  }
 }

src/main/menu.file.ts

@@ -1,13 +1,17 @@
+import { BrowserWindow, MenuItemConstructorOptions } from "electron";
+
 import { I18nService } from "jslib-common/abstractions/i18n.service";
 import { MessagingService } from "jslib-common/abstractions/messaging.service";
 
-import { isMacAppStore } from "jslib-electron/utils";
-
 import { IMenubarMenu } from "./menubar";
 
-import { MenuItemConstructorOptions } from "electron";
+import { FirstMenu } from "./menu.first";
+import { MenuAccount } from "./menu.updater";
 
-export class FileMenu implements IMenubarMenu {
+import { UpdaterMain } from "jslib-electron/updater.main";
+import { isMac, isMacAppStore } from "jslib-electron/utils";
+
+export class FileMenu extends FirstMenu implements IMenubarMenu {
   readonly id: string = "fileMenu";
 
   get label(): string {
@@ -15,25 +19,42 @@ export class FileMenu implements IMenubarMenu {
   }
 
   get items(): MenuItemConstructorOptions[] {
-    return [
+    let items = [
       this.addNewLogin,
       this.addNewItem,
       this.addNewFolder,
       this.separator,
       this.syncVault,
       this.exportVault,
-      this.quitBitwarden,
     ];
+
+    if (!isMac()) {
+      items = [
+        ...items,
+        ...[
+          this.separator,
+          this.settings,
+          this.lock,
+          this.lockAll,
+          this.logOut,
+          this.separator,
+          this.quitBitwarden,
+        ],
+      ];
+    }
+
+    return items;
   }
 
-  private readonly _i18nService: I18nService;
+  constructor(
-  private readonly _messagingService: MessagingService;
+    i18nService: I18nService,
-  private readonly _isLocked: boolean;
+    messagingService: MessagingService,
-
+    updater: UpdaterMain,
-  constructor(i18nService: I18nService, messagingService: MessagingService, isLocked: boolean) {
+    window: BrowserWindow,
-    this._i18nService = i18nService;
+    accounts: { [userId: string]: MenuAccount },
-    this._messagingService = messagingService;
+    isLocked: boolean
-    this._isLocked = isLocked;
+  ) {
+    super(i18nService, messagingService, updater, window, accounts, isLocked);
   }
 
   private get addNewLogin(): MenuItemConstructorOptions {
@@ -93,10 +114,6 @@ export class FileMenu implements IMenubarMenu {
     };
   }
 
-  private get separator(): MenuItemConstructorOptions {
-    return { type: "separator" };
-  }
-
   private get syncVault(): MenuItemConstructorOptions {
     return {
       id: "syncVault",
@@ -123,12 +140,4 @@ export class FileMenu implements IMenubarMenu {
       role: "quit",
     };
   }
-
-  private localize(s: string) {
-    return this._i18nService.t(s);
-  }
-
-  private sendMessage(message: string) {
-    this._messagingService.send(message);
-  }
 }

src/main/menu.first.ts

@@ -0,0 +1,153 @@
+import { BrowserWindow, dialog, MenuItem, MenuItemConstructorOptions } from "electron";
+
+import { I18nService } from "jslib-common/abstractions/i18n.service";
+import { MessagingService } from "jslib-common/abstractions/messaging.service";
+
+import { UpdaterMain } from "jslib-electron/updater.main";
+import { isMacAppStore, isSnapStore, isWindowsStore } from "jslib-electron/utils";
+
+import { MenuAccount } from "./menu.updater";
+
+export class FirstMenu {
+  protected readonly _i18nService: I18nService;
+  protected readonly _updater: UpdaterMain;
+  protected readonly _messagingService: MessagingService;
+  protected readonly _accounts: { [userId: string]: MenuAccount };
+  protected readonly _window: BrowserWindow;
+  protected readonly _isLocked: boolean;
+
+  constructor(
+    i18nService: I18nService,
+    messagingService: MessagingService,
+    updater: UpdaterMain,
+    window: BrowserWindow,
+    accounts: { [userId: string]: MenuAccount },
+    isLocked: boolean
+  ) {
+    this._i18nService = i18nService;
+    this._updater = updater;
+    this._messagingService = messagingService;
+    this._window = window;
+    this._accounts = accounts;
+    this._isLocked = isLocked;
+  }
+
+  protected get hasAccounts(): boolean {
+    return this._accounts != null && Object.keys(this._accounts).length > 0;
+  }
+
+  protected get checkForUpdates(): MenuItemConstructorOptions {
+    return {
+      id: "checkForUpdates",
+      label: this.localize("checkForUpdates"),
+      click: (menuItem) => this.checkForUpdate(menuItem),
+      visible: !isMacAppStore() && !isWindowsStore() && !isSnapStore(),
+    };
+  }
+
+  protected get separator(): MenuItemConstructorOptions {
+    return {
+      type: "separator",
+    };
+  }
+
+  protected get settings(): MenuItemConstructorOptions {
+    return {
+      id: "settings",
+      label: this.localize(process.platform === "darwin" ? "preferences" : "settings"),
+      click: () => this.sendMessage("openSettings"),
+      accelerator: "CmdOrCtrl+,",
+      enabled: !this._isLocked,
+    };
+  }
+
+  protected get lock(): MenuItemConstructorOptions {
+    return {
+      id: "lock",
+      label: this.localize("lockVault"),
+      submenu: this.lockSubmenu,
+      enabled: this.hasAccounts,
+    };
+  }
+
+  protected get lockSubmenu(): MenuItemConstructorOptions[] {
+    const value: MenuItemConstructorOptions[] = [];
+    for (const userId in this._accounts) {
+      if (userId == null) {
+        continue;
+      }
+
+      value.push({
+        label: this._accounts[userId].email,
+        id: `lockNow_${this._accounts[userId].userId}`,
+        click: () => this.sendMessage("lockVault", { userId: this._accounts[userId].userId }),
+        enabled: !this._accounts[userId].isLocked,
+        visible: this._accounts[userId].isAuthenticated,
+      });
+    }
+    return value;
+  }
+
+  protected get lockAll(): MenuItemConstructorOptions {
+    return {
+      id: "lockAllNow",
+      label: this.localize("lockAllVaults"),
+      click: () => this.sendMessage("lockAllVaults"),
+      accelerator: "CmdOrCtrl+L",
+      enabled: this.hasAccounts,
+    };
+  }
+
+  protected get logOut(): MenuItemConstructorOptions {
+    return {
+      id: "logOut",
+      label: this.localize("logOut"),
+      submenu: this.logOutSubmenu,
+      enabled: this.hasAccounts,
+    };
+  }
+
+  protected get logOutSubmenu(): MenuItemConstructorOptions[] {
+    const value: MenuItemConstructorOptions[] = [];
+    for (const userId in this._accounts) {
+      if (userId == null) {
+        continue;
+      }
+
+      value.push({
+        label: this._accounts[userId].email,
+        id: `logOut_${this._accounts[userId].userId}`,
+        click: async () => {
+          const result = await dialog.showMessageBox(this._window, {
+            title: this.localize("logOut"),
+            message: this.localize("logOut"),
+            detail: this.localize("logOutConfirmation"),
+            buttons: [this.localize("logOut"), this.localize("cancel")],
+            cancelId: 1,
+            defaultId: 0,
+            noLink: true,
+          });
+          if (result.response === 0) {
+            this.sendMessage("logout", { userId: this._accounts[userId].userId });
+          }
+        },
+        visible: this._accounts[userId].isAuthenticated,
+      });
+    }
+    return value;
+  }
+
+  protected localize(s: string) {
+    return this._i18nService.t(s);
+  }
+
+  protected async checkForUpdate(menuItem: MenuItem) {
+    menuItem.enabled = false;
+    this._updater.checkForUpdate(true);
+    menuItem.enabled = true;
+  }
+
+  protected sendMessage(message: string, args?: any) {
+    this._messagingService.send(message, args);
+  }
+}

src/main/menu.help.ts

@@ -74,7 +74,7 @@ export class HelpMenu implements IMenubarMenu {
     return {
       id: "legal",
       label: this.localize("legal"),
-      visible: !isMacAppStore(),
+      visible: isMacAppStore(),
       submenu: this.legalSubmenu,
     };
   }

src/main/menu.window.ts

@@ -1,7 +1,7 @@
 import { I18nService } from "jslib-common/abstractions/i18n.service";
 import { MessagingService } from "jslib-common/abstractions/messaging.service";
 
-import { isMacAppStore } from "jslib-electron/utils";
+import { isMac } from "jslib-electron/utils";
 import { WindowMain } from "jslib-electron/window.main";
 
 import { IMenubarMenu } from "./menubar";
@@ -16,19 +16,14 @@ export class WindowMenu implements IMenubarMenu {
   }
 
   get items(): MenuItemConstructorOptions[] {
-    if (!isMacAppStore()) {
+    const items = [this.minimize, this.hideToMenu, this.alwaysOnTop];
-      return [this.hideToMenu, this.alwaysOnTop];
+
+    if (isMac()) {
+      items.concat([this.zoom, this.separator, this.bringAllToFront]);
     }
 
-    return [
+    items.concat([this.separator, this.close]);
-      this.minimize,
+    return items;
-      this.hideToMenu,
-      this.alwaysOnTop,
-      this.zoom,
-      this.separator,
-      this.bringAllToFront,
-      this.close,
-    ];
   }
 
   private readonly _i18nService: I18nService;
@@ -50,14 +45,13 @@ export class WindowMenu implements IMenubarMenu {
       id: "minimize",
       label: this.localize("minimize"),
       role: "minimize",
-      visible: isMacAppStore(),
     };
   }
 
   private get hideToMenu(): MenuItemConstructorOptions {
     return {
       id: "hideToMenu",
-      label: this.localize(isMacAppStore() ? "hideToMenuBar" : "hideToTray"),
+      label: this.localize(isMac() ? "hideToMenuBar" : "hideToTray"),
       click: () => this.sendMessage("hideToTray"),
       accelerator: "CmdOrCtrl+Shift+M",
     };
@@ -79,7 +73,6 @@ export class WindowMenu implements IMenubarMenu {
       id: "zoom",
       label: this.localize("zoom"),
       role: "zoom",
-      visible: isMacAppStore(),
     };
   }
 
@@ -92,7 +85,6 @@ export class WindowMenu implements IMenubarMenu {
       id: "bringAllToFront",
       label: this.localize("bringAllToFront"),
       role: "front",
-      visible: isMacAppStore(),
     };
   }
 
@@ -101,7 +93,6 @@ export class WindowMenu implements IMenubarMenu {
       id: "close",
       label: this.localize("close"),
       role: "close",
-      visible: isMacAppStore(),
     };
   }
 

src/main/menubar.ts

@@ -14,6 +14,7 @@ import { I18nService } from "jslib-common/abstractions/i18n.service";
 import { MessagingService } from "jslib-common/abstractions/messaging.service";
 
 import { UpdaterMain } from "jslib-electron/updater.main";
+import { isMac } from "jslib-electron/utils";
 import { WindowMain } from "jslib-electron/window.main";
 
 export interface IMenubarMenu {
@@ -62,7 +63,7 @@ export class Menubar {
     }
 
     this.items = [
-      new BitwardenMenu(
+      new FileMenu(
         i18nService,
         messagingService,
         updaterMain,
@@ -70,7 +71,6 @@ export class Menubar {
         updateRequest?.accounts,
         isLocked
       ),
-      new FileMenu(i18nService, messagingService, isLocked),
       new EditMenu(i18nService, messagingService, isLocked),
       new ViewMenu(i18nService, messagingService, isLocked),
       new AccountMenu(i18nService, messagingService, webVaultUrl, windowMain.win, isLocked),
@@ -81,5 +81,21 @@ export class Menubar {
         new AboutMenu(i18nService, appVersion, windowMain.win, updaterMain)
       ),
     ];
+
+    if (isMac()) {
+      this.items = [
+        ...[
+          new BitwardenMenu(
+            i18nService,
+            messagingService,
+            updaterMain,
+            windowMain.win,
+            updateRequest?.accounts,
+            isLocked
+          ),
+        ],
+        ...this.items,
+      ];
+    }
   }
 }