Skip to content

Commit

Permalink
Fix duplicate menu items in Window menu
Browse files Browse the repository at this point in the history
Previously MacVim would see a lot of duplicate window menu items like
"Enter Full Screen" or "Tile Window to Left of Screen" when the user
toggles between two windows. This is because the `setWindowsMenu:` call
was injecting these items, but AppKit isn't smart enough to de-duplicate
them (unlike the window list at the bottom). Just fix this by making a
copy of the main menu before passing it in. This way every time we try
to set a main menu (which happens whenever we jump among Vim windows as
each Vim can have different menu items), it will be set with a fresh one
that doesn't have the injected menu items in it.

Also, set NSFullScreenMenuItemEverywhere to prevent AppKit from
injecting "Enter Full Screen" items. MacVim already has similar menu
items to handle that.

Also, remove old private API call to `setAppleMenu:`. As far as I could
tell this is not useful anymore in recent macOS versions and that line
of code was written in 2008.

Fix macvim-dev#566, Fix macvim-dev#992
  • Loading branch information
ychin committed Sep 18, 2020
1 parent 8b25779 commit 2828a57
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/MacVim/MMAppController.h
Expand Up @@ -23,6 +23,7 @@
NSString *openSelectionString;
NSMutableDictionary *pidArguments;
NSMenu *defaultMainMenu;
NSMenu *currentMainMenu;
NSMenuItem *appMenuItemTemplate;
NSMenuItem *recentFilesMenuItem;
NSMutableArray *cachedVimControllers;
Expand Down
50 changes: 22 additions & 28 deletions src/MacVim/MMAppController.m
Expand Up @@ -321,6 +321,7 @@ - (void)dealloc
[openSelectionString release]; openSelectionString = nil;
[recentFilesMenuItem release]; recentFilesMenuItem = nil;
[defaultMainMenu release]; defaultMainMenu = nil;
currentMainMenu = nil;
[appMenuItemTemplate release]; appMenuItemTemplate = nil;
[updater release]; updater = nil;

Expand All @@ -329,6 +330,11 @@ - (void)dealloc

- (void)applicationWillFinishLaunching:(NSNotification *)notification
{
// This prevents macOS from injecting "Enter Full Screen" menu item.
// MacVim already has a separate menu item to do that.
// See https://developer.apple.com/library/archive/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_11FullScreen
[[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"NSFullScreenMenuItemEverywhere"];

// Remember the default menu so that it can be restored if the user closes
// all editor windows.
defaultMainMenu = [[NSApp mainMenu] retain];
Expand Down Expand Up @@ -862,7 +868,14 @@ - (void)windowControllerWillOpen:(MMWindowController *)windowController

- (void)setMainMenu:(NSMenu *)mainMenu
{
if ([NSApp mainMenu] == mainMenu) return;
if (currentMainMenu == mainMenu) return;
currentMainMenu = mainMenu;

// Make a copy of the menu before we pass to AppKit. The main reason is
// that setWindowsMenu: below will inject items like "Tile Window to Left
// of Screen" to the Window menu, and on repeated calls it will keep adding
// the same item over and over again, without resolving for duplicates.
mainMenu = [mainMenu copy];

// If the new menu has a "Recent Files" dummy item, then swap the real item
// for the dummy. We are forced to do this since Cocoa initializes the
Expand All @@ -889,45 +902,26 @@ - (void)setMainMenu:(NSMenu *)mainMenu
}
}

// Now set the new menu. Notice that we keep one menu for each editor
// window since each editor can have its own set of menus. When swapping
// menus we have to tell Cocoa where the new "MacVim", "Windows", and
// "Services" menu are.
[NSApp setMainMenu:mainMenu];

// Setting the "MacVim" (or "Application") menu ensures that it is typeset
// in boldface. (The setAppleMenu: method used to be public but is now
// private so this will have to be considered a bit of a hack!)
NSMenu *appMenu = [mainMenu findApplicationMenu];
[NSApp performSelector:@selector(setAppleMenu:) withObject:appMenu];

#if DISABLE_SPARKLE
NSMenu *appMenu = [mainMenu findApplicationMenu];

// If Sparkle is disabled, we want to remove the "Check for Updates" menu
// item since it's no longer useful.
NSMenuItem *checkForUpdatesItem = [appMenu itemAtIndex:
[appMenu indexOfItemWithAction:@selector(checkForUpdates:)]];
checkForUpdatesItem.hidden = true;
#endif

// Now set the new menu. Notice that we keep one menu for each editor
// window since each editor can have its own set of menus. When swapping
// menus we have to tell Cocoa where the new "MacVim", "Windows", and
// "Services" menu are.
[NSApp setMainMenu:mainMenu];

NSMenu *servicesMenu = [mainMenu findServicesMenu];
[NSApp setServicesMenu:servicesMenu];

NSMenu *windowsMenu = [mainMenu findWindowsMenu];
if (windowsMenu) {
// Cocoa isn't clever enough to get rid of items it has added to the
// "Windows" menu so we have to do it ourselves otherwise there will be
// multiple menu items for each window in the "Windows" menu.
// This code assumes that the only items Cocoa add are ones which
// send off the action makeKeyAndOrderFront:. (Cocoa will not add
// another separator item if the last item on the "Windows" menu
// already is a separator, so we needen't worry about separators.)
int i, count = [windowsMenu numberOfItems];
for (i = count-1; i >= 0; --i) {
NSMenuItem *item = [windowsMenu itemAtIndex:i];
if ([item action] == @selector(makeKeyAndOrderFront:))
[windowsMenu removeItem:item];
}
}
[NSApp setWindowsMenu:windowsMenu];
}

Expand Down

0 comments on commit 2828a57

Please sign in to comment.