From 4126de887799c61793bf1f9efc8b7ac7b66c8b32 Mon Sep 17 00:00:00 2001
From: Gabriel de Dietrich <gabriel.dedietrich@qt.io>
Date: Tue, 24 Jul 2018 13:23:35 -0700
Subject: QCocoaMenuLoader - ensure that ensureAppMenuInMenu indeed, ensures

The logic seems to be incorrect (or the naming is misleading): it
only adds 'appMenu' if it was found in the previous 'mainMenu',
failing otherwise. Consider the following example:

while (true){
    QApplication app(a,b);
    MainWindow w;
    w.show();
    app.exec();
}

It's quite a contrived but apparently allowed API use (OP claims
they have to switch languages in their app). The main window and the
app are destroyed, so is the menu bar. Then a new main window is
created, with a new menu bar. Now the current [NSApp mainMenu]
(the one set after we deleted the previous) does not have 'appMenu'
anymore (we removed it when initializing the first menu bar).
So as a result we have app menu missing and add new menus/items
to a wrong menus/at wrong index.

Change-Id: I64fce766d6c12ebf7ae12bb94af41c8c1de3d78b
Task-number: QTBUG-69496
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
---
 src/plugins/platforms/cocoa/qcocoamenuloader.mm | 33 +++++++++++--------------
 1 file changed, 15 insertions(+), 18 deletions(-)

(limited to 'src/plugins')

diff --git a/src/plugins/platforms/cocoa/qcocoamenuloader.mm b/src/plugins/platforms/cocoa/qcocoamenuloader.mm
index cd597da71c..4432d3e27a 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuloader.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuloader.mm
@@ -191,26 +191,23 @@
     Q_ASSERT(mainMenu);
 #endif
     // Grab the app menu out of the current menu.
-    int numItems = [mainMenu numberOfItems];
-    NSMenuItem *oldAppMenuItem = 0;
-    for (int i = 0; i < numItems; ++i) {
-        NSMenuItem *item = [mainMenu itemAtIndex:i];
-        if ([item submenu] == appMenu) {
-            oldAppMenuItem = item;
-            [oldAppMenuItem retain];
-            [mainMenu removeItemAtIndex:i];
-            break;
+    auto unparentAppMenu = ^bool (NSMenu *supermenu) {
+        auto index = [supermenu indexOfItemWithSubmenu:appMenu];
+        if (index != -1) {
+            [supermenu removeItemAtIndex:index];
+            return true;
         }
-    }
+        return false;
+    };
 
-    if (oldAppMenuItem) {
-        [oldAppMenuItem setSubmenu:nil];
-        [oldAppMenuItem release];
-        NSMenuItem *appMenuItem = [[NSMenuItem alloc] initWithTitle:@"Apple"
-            action:nil keyEquivalent:@""];
-        [appMenuItem setSubmenu:appMenu];
-        [menu insertItem:appMenuItem atIndex:0];
-    }
+    if (!mainMenu || !unparentAppMenu(mainMenu))
+        if (appMenu.supermenu)
+            unparentAppMenu(appMenu.supermenu);
+
+    NSMenuItem *appMenuItem = [[NSMenuItem alloc] initWithTitle:@"Apple"
+        action:nil keyEquivalent:@""];
+    [appMenuItem setSubmenu:appMenu];
+    [menu insertItem:appMenuItem atIndex:0];
 }
 
 - (void)removeActionsFromAppMenu
-- 
cgit v1.2.3