Skip to content

Commit 2828a57

Browse files
committed
Fix duplicate menu items in Window menu
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 #566, Fix #992
1 parent 8b25779 commit 2828a57

File tree

2 files changed

+23
-28
lines changed

2 files changed

+23
-28
lines changed

src/MacVim/MMAppController.h

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
NSString *openSelectionString;
2424
NSMutableDictionary *pidArguments;
2525
NSMenu *defaultMainMenu;
26+
NSMenu *currentMainMenu;
2627
NSMenuItem *appMenuItemTemplate;
2728
NSMenuItem *recentFilesMenuItem;
2829
NSMutableArray *cachedVimControllers;

src/MacVim/MMAppController.m

+22-28
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ - (void)dealloc
321321
[openSelectionString release]; openSelectionString = nil;
322322
[recentFilesMenuItem release]; recentFilesMenuItem = nil;
323323
[defaultMainMenu release]; defaultMainMenu = nil;
324+
currentMainMenu = nil;
324325
[appMenuItemTemplate release]; appMenuItemTemplate = nil;
325326
[updater release]; updater = nil;
326327

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

330331
- (void)applicationWillFinishLaunching:(NSNotification *)notification
331332
{
333+
// This prevents macOS from injecting "Enter Full Screen" menu item.
334+
// MacVim already has a separate menu item to do that.
335+
// See https://developer.apple.com/library/archive/releasenotes/AppKit/RN-AppKitOlderNotes/index.html#10_11FullScreen
336+
[[NSUserDefaults standardUserDefaults] setBool:NO forKey:@"NSFullScreenMenuItemEverywhere"];
337+
332338
// Remember the default menu so that it can be restored if the user closes
333339
// all editor windows.
334340
defaultMainMenu = [[NSApp mainMenu] retain];
@@ -862,7 +868,14 @@ - (void)windowControllerWillOpen:(MMWindowController *)windowController
862868

863869
- (void)setMainMenu:(NSMenu *)mainMenu
864870
{
865-
if ([NSApp mainMenu] == mainMenu) return;
871+
if (currentMainMenu == mainMenu) return;
872+
currentMainMenu = mainMenu;
873+
874+
// Make a copy of the menu before we pass to AppKit. The main reason is
875+
// that setWindowsMenu: below will inject items like "Tile Window to Left
876+
// of Screen" to the Window menu, and on repeated calls it will keep adding
877+
// the same item over and over again, without resolving for duplicates.
878+
mainMenu = [mainMenu copy];
866879

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

892-
// Now set the new menu. Notice that we keep one menu for each editor
893-
// window since each editor can have its own set of menus. When swapping
894-
// menus we have to tell Cocoa where the new "MacVim", "Windows", and
895-
// "Services" menu are.
896-
[NSApp setMainMenu:mainMenu];
897-
898-
// Setting the "MacVim" (or "Application") menu ensures that it is typeset
899-
// in boldface. (The setAppleMenu: method used to be public but is now
900-
// private so this will have to be considered a bit of a hack!)
901-
NSMenu *appMenu = [mainMenu findApplicationMenu];
902-
[NSApp performSelector:@selector(setAppleMenu:) withObject:appMenu];
903-
904905
#if DISABLE_SPARKLE
906+
NSMenu *appMenu = [mainMenu findApplicationMenu];
907+
905908
// If Sparkle is disabled, we want to remove the "Check for Updates" menu
906909
// item since it's no longer useful.
907910
NSMenuItem *checkForUpdatesItem = [appMenu itemAtIndex:
908911
[appMenu indexOfItemWithAction:@selector(checkForUpdates:)]];
909912
checkForUpdatesItem.hidden = true;
910913
#endif
911914

915+
// Now set the new menu. Notice that we keep one menu for each editor
916+
// window since each editor can have its own set of menus. When swapping
917+
// menus we have to tell Cocoa where the new "MacVim", "Windows", and
918+
// "Services" menu are.
919+
[NSApp setMainMenu:mainMenu];
920+
912921
NSMenu *servicesMenu = [mainMenu findServicesMenu];
913922
[NSApp setServicesMenu:servicesMenu];
914923

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

0 commit comments

Comments
 (0)