Skip to content
This repository has been archived by the owner on Jul 23, 2022. It is now read-only.

cmd + w doesn't close the window on macOS #714

Closed
jonohewitt opened this issue Feb 6, 2021 · 7 comments · Fixed by #836
Closed

cmd + w doesn't close the window on macOS #714

jonohewitt opened this issue Feb 6, 2021 · 7 comments · Fixed by #836

Comments

@jonohewitt
Copy link

Describe the Bug

cmd + w doesn't close the window on macOS

Steps To Reproduce

  1. Open Bitwarden
  2. Press cmd + w

Expected Result

The window should close but the application should not quit

Actual Result

No effect

Screenshots or Videos

Screenshot 2021-02-06 at 16 51 27

Environment

  • MacOS Big Sur 11.2
  • Build Version 1.24.5 (513)

Additional Context

Appears to have started since adding new settings options in relation to #419

@ffeu
Copy link

ffeu commented Feb 25, 2021

Hi, just wanted to point that this works in Catalina 10.15.7 and BitWarden Version 1.24.6 (515) - installed it through brew. So it may be Big Sur specific.

@CarlAndersson
Copy link

Hi, I'm on Catalina 10.15.7 and cmd+w does not work for me.
I'm using the same settings as @jonohewitt but otherwise a fresh install of the app.

@cho-m
Copy link

cho-m commented Mar 24, 2021

I commented on a duplicate issue #802 (comment) that there is an implemented behavior difference between DMG (via Brew/homepage) and MAS versions.

Images showing difference in menu options copied from that post:

DMG Menu MAS Menu
dmg-menu mas-menu

The fix may be simply removing the MAS-check, but I am not sure if there is an easy way to locally build/test the MAS version.
Possible diff (for bitwarden/jslib):

diff --git a/src/electron/baseMenu.ts b/src/electron/baseMenu.ts
index de08e4d..2523e63 100644
--- a/src/electron/baseMenu.ts
+++ b/src/electron/baseMenu.ts
@@ -9,8 +9,6 @@ import {
 import { I18nService } from '../abstractions/i18n.service';
 import { WindowMain } from './window.main';

-import { isMacAppStore } from './utils';
-
 export class BaseMenu {
     protected editMenuItemOptions: MenuItemConstructorOptions;
     protected viewSubMenuItemOptions: MenuItemConstructorOptions[];
@@ -143,7 +141,7 @@ export class BaseMenu {
                 },
                 {
                     label: this.i18nService.t('close'),
-                    role: isMacAppStore() ? 'quit' : 'close',
+                    role: 'close',
                 },
             ];
         }

@cscharf
Copy link
Contributor

cscharf commented Mar 30, 2021

Okay, so funny story @cho-m , that change was implemented back then because Apple originally rejected the app from the app store because it used the close action instead of quit... it was the original submission review and perhaps we can just assume they put a rookie on the new app review duty or what, but Kyle had to change that in order to get the app approved (which definitely doesn't make any sense).

Either way, we can change it and we're open to it being change back to remove the conditional and just leave it as close for the role assignment for that menu item. We'll find out one way or the other on MAS store review whether they reject it again for the same reason (which would definitely leave us scratching our heads at this point).

@kapitainsky
Copy link

quick question - has it been already taken care of? Or you require PR to move it forward?

I have tried no MAS version and it supports both ⌘Q for application and ⌘W for window so based on @cscharf description it seems like some Apple approver mistake indeed.

@cscharf
Copy link
Contributor

cscharf commented Apr 1, 2021

quick question - has it been already taken care of? Or you require PR to move it forward?

It requires a PR, either from the community or when one of our engineers has an opportunity to do so.

@kapitainsky
Copy link

ta. Will try to create one over Easter then. It is small but annoying bug on macOS.

kapitainsky added a commit to kapitainsky/jslib that referenced this issue Apr 13, 2021
cmd + w doesn't close the window on macOS. Based on @cscharf explanation - bitwarden/desktop#714 (comment) - it seems that issue was introduced in response to erroneous Apple Store approver request. The code change reversed this change.
cscharf pushed a commit to bitwarden/jslib that referenced this issue Apr 13, 2021
cmd + w doesn't close the window on macOS. Based on @cscharf explanation - bitwarden/desktop#714 (comment) - it seems that issue was introduced in response to erroneous Apple Store approver request. The code change reversed this change.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants