QCocoaMenuLoader: get rid of lastAppSpecificItem

Look it up when needed instead. Also, simplify our
ownership logic - do not retain/autorelease that
is already owned by a menu (via its itemArray).

Fixes: QTBUG-76523
Change-Id: I60a2ed0d192396baf99eec7b37fa5cc10e5db626
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This commit is contained in:
Timur Pocheptsov 2019-08-07 13:33:41 +02:00
parent afb326f071
commit aef0fba3c5

View File

@ -59,7 +59,6 @@
NSMenuItem *aboutItem; NSMenuItem *aboutItem;
NSMenuItem *aboutQtItem; NSMenuItem *aboutQtItem;
NSMenuItem *hideItem; NSMenuItem *hideItem;
NSMenuItem *lastAppSpecificItem;
NSMenuItem *servicesItem; NSMenuItem *servicesItem;
NSMenuItem *hideAllOthersItem; NSMenuItem *hideAllOthersItem;
NSMenuItem *showAllItem; NSMenuItem *showAllItem;
@ -118,6 +117,9 @@
[appMenu addItem:[NSMenuItem separatorItem]]; [appMenu addItem:[NSMenuItem separatorItem]];
// Preferences // Preferences
// We'll be adding app specific items after this. The macOS HIG state that,
// "In general, a Preferences menu item should be the first app-specific menu item."
// https://developer.apple.com/macos/human-interface-guidelines/menus/menu-bar-menus/
preferencesItem = [[QCocoaNSMenuItem alloc] init]; preferencesItem = [[QCocoaNSMenuItem alloc] init];
preferencesItem.title = @"Preferences…"; preferencesItem.title = @"Preferences…";
preferencesItem.keyEquivalent = @","; preferencesItem.keyEquivalent = @",";
@ -126,11 +128,6 @@
preferencesItem.hidden = YES; preferencesItem.hidden = YES;
[appMenu addItem:preferencesItem]; [appMenu addItem:preferencesItem];
// We'll be adding app specific items after this. The macOS HIG state that,
// "In general, a Preferences menu item should be the first app-specific menu item."
// https://developer.apple.com/macos/human-interface-guidelines/menus/menu-bar-menus/
lastAppSpecificItem = preferencesItem;
[appMenu addItem:[NSMenuItem separatorItem]]; [appMenu addItem:[NSMenuItem separatorItem]];
// Services item and menu // Services item and menu
@ -194,8 +191,6 @@
[showAllItem release]; [showAllItem release];
[quitItem release]; [quitItem release];
[lastAppSpecificItem release];
[super dealloc]; [super dealloc];
} }
@ -272,25 +267,20 @@
// No reason to create the item if it already exists. // No reason to create the item if it already exists.
for (NSMenuItem *item in appMenu.itemArray) for (NSMenuItem *item in appMenu.itemArray)
if (qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem == platformItem) if (qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem == platformItem)
return [[item retain] autorelease]; return item;
// Create an App-Specific menu item, insert it into the menu and return // Create an App-Specific menu item, insert it into the menu and return
// it as an autorelease item. // it as an autorelease item.
QCocoaNSMenuItem *item; QCocoaNSMenuItem *item;
if (platformItem->isSeparator()) if (platformItem->isSeparator())
item = [[QCocoaNSMenuItem separatorItemWithPlatformMenuItem:platformItem] retain]; item = [QCocoaNSMenuItem separatorItemWithPlatformMenuItem:platformItem];
else else
item = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:platformItem]; item = [[[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:platformItem] autorelease];
const auto location = [appMenu indexOfItem:lastAppSpecificItem]; const auto location = [self indexOfLastAppSpecificMenuItem];
[appMenu insertItem:item atIndex:NSInteger(location) + 1];
if (!lastAppSpecificItem.separatorItem) return item;
[lastAppSpecificItem release];
lastAppSpecificItem = item; // Keep track of this for later (i.e., don't release it)
[appMenu insertItem:item atIndex:location + 1];
return [[item retain] autorelease];
} }
- (void)orderFrontStandardAboutPanel:(id)sender - (void)orderFrontStandardAboutPanel:(id)sender
@ -344,8 +334,24 @@
- (NSArray<NSMenuItem *> *)mergeable - (NSArray<NSMenuItem *> *)mergeable
{ {
// Don't include the quitItem here, since we want it always visible and enabled regardless // Don't include the quitItem here, since we want it always visible and enabled regardless
// Note that lastAppSpecificItem may be nil, so we can't use @[] here. auto items = [NSArray arrayWithObjects:preferencesItem, aboutItem, aboutQtItem,
return [NSArray arrayWithObjects:preferencesItem, aboutItem, aboutQtItem, lastAppSpecificItem, nil]; appMenu.itemArray[[self indexOfLastAppSpecificMenuItem]], nil];
return items;
} }
- (NSUInteger)indexOfLastAppSpecificMenuItem
{
// Either the 'Preferences', which is the first app specific menu item, or something
// else we appended later (thus the reverse order):
const auto location = [appMenu.itemArray indexOfObjectWithOptions:NSEnumerationReverse
passingTest:^BOOL(NSMenuItem *item, NSUInteger, BOOL *) {
if (auto qtItem = qt_objc_cast<QCocoaNSMenuItem*>(item))
return qtItem != quitItem;
return NO;
}];
Q_ASSERT(location != NSNotFound);
return location;
}
@end @end