QCocoaMenu: Decouple NSMenuItem from NSMenu

While Cocoa requires an NSMenu to be coupled to an NSMenuItem
(just as Qt requires a QMenu to be coupled to a QAction), making
that a hard coupling comes with some limitations. This is because
Cocoa won't allow the NSMenu object to be simultaneously coupled
to more than one NSMenuItem and, similarly, an NSMenuItem can
only be added to a single parent NSMenu. Therefore, it becomes
difficult to share one QMenu between two different QMenuBars in
different windows, or to use a QMenu as context menu while being
accessible from the menu bar.

Previous solutions to circumvent those limitations were less than
ideal (see 119882714f87ffeb6945fdb2d02997ae125ff50c for the
QMenuBar shared QMenu issue). Other workarounds that relied on
that hard coupling, like 996054f5e65bc676aaea0743c2eacec51918e4aa,
also added gratuitous complexity.

In this patch, we break that hard NSMenuItem-NSMenu coupling, and
we replace it with a temporary, looser coupling. As a consequence,

   * QCocoaMenu only contains and manages a NSMenu instance,
     removing the previously used NSMenuItem. It gets a temporarily
     attached NSMenuItem instead.
   * QCocoaMenuItem gains a safe pointer to its QCocoaMenu property
     removing the necessity containingMenuItem() in QCocoaMenu.
   * QCocoaMenuBar manages its own NSMenuItems.

With this setup, we bind the NSMenu to its parent NSMenuItem at the
last moment. In QCocoaMenuBar, when we call updateMenuBarImmediately().
In QCocoaMenu, we use the delegate's -[QCocoaMenuDelegate menu:
updateItem:atIndex:shouldCancel:] method which is called when Cocoa
is about to display the NSMenu.

Note: There's still one use case we don't support, which is sharing
a toplevel QMenuBar menu. This is because Cocoa's menu bar requires
each of its menu items to have a submenu assigned, and therefore we
can't rely on that last moment assignment.

Task-number: QTBUG-34160
Task-number: QTBUG-31342
Task-number: QTBUG-41587
Change-Id: I92bdb444c680789c78e43fe0b585dc6661770281
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@theqtcompany.com>

1 files changed, 42 insertions, 36 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index ad491855ff..21a3799859 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -96,6 +96,28 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaMenuDelegate);
     return self;
 }
 
+- (NSInteger)numberOfItemsInMenu:(NSMenu *)menu
+{
+    Q_ASSERT(m_menu->nsMenu() == menu);
+    return m_menu->items().count();
+}
+
+- (BOOL)menu:(NSMenu *)menu updateItem:(NSMenuItem *)item atIndex:(NSInteger)index shouldCancel:(BOOL)shouldCancel
+{
+    Q_UNUSED(index);
+    Q_ASSERT(m_menu->nsMenu() == menu);
+    if (shouldCancel) {
+        // TODO detach all submenus
+        return NO;
+    }
+
+    QCocoaMenuItem *menuItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
+    if (m_menu->items().contains(menuItem)) {
+        if (QCocoaMenu *itemSubmenu = menuItem->menu())
+            itemSubmenu->setAttachedItem(item);
+    }
+    return YES;
+}
 
 - (void)menu:(NSMenu*)menu willHighlightItem:(NSMenuItem*)item
 {
@@ -228,20 +250,16 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaMenuDelegate);
 QT_BEGIN_NAMESPACE
 
 QCocoaMenu::QCocoaMenu() :
+    m_attachedItem(0),
     m_enabled(true),
     m_visible(true),
-    m_tag(0),
-    m_menuBar(0),
-    m_containingMenuItem(0)
+    m_tag(0)
 {
     QMacAutoReleasePool pool;
 
-    m_delegate = [[QCocoaMenuDelegate alloc] initWithMenu:this];
-    m_nativeItem = [[NSMenuItem alloc] initWithTitle:@"" action:nil keyEquivalent:@""];
     m_nativeMenu = [[NSMenu alloc] initWithTitle:@"Untitled"];
     [m_nativeMenu setAutoenablesItems:YES];
-    m_nativeMenu.delegate = (QCocoaMenuDelegate *) m_delegate;
-    [m_nativeItem setSubmenu:m_nativeMenu];
+    m_nativeMenu.delegate = [[QCocoaMenuDelegate alloc] initWithMenu:this];
 }
 
 QCocoaMenu::~QCocoaMenu()
@@ -251,14 +269,11 @@ QCocoaMenu::~QCocoaMenu()
             SET_COCOA_MENU_ANCESTOR(item, 0);
     }
 
-    if (m_containingMenuItem)
-        m_containingMenuItem->clearMenu(this);
-
     QMacAutoReleasePool pool;
-    [m_nativeItem setSubmenu:nil];
+    NSObject *delegate = m_nativeMenu.delegate;
+    m_nativeMenu.delegate = nil;
+    [delegate release];
     [m_nativeMenu release];
-    [m_delegate release];
-    [m_nativeItem release];
 }
 
 void QCocoaMenu::setText(const QString &text)
@@ -266,7 +281,6 @@ void QCocoaMenu::setText(const QString &text)
     QMacAutoReleasePool pool;
     QString stripped = qt_mac_removeAmpersandEscapes(text);
     [m_nativeMenu setTitle:QCFString::toNSString(stripped)];
-    [m_nativeItem setTitle:QCFString::toNSString(stripped)];
 }
 
 void QCocoaMenu::setMinimumWidth(int width)
@@ -307,17 +321,13 @@ void QCocoaMenu::insertMenuItem(QPlatformMenuItem *menuItem, QPlatformMenuItem *
 
 void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
 {
-    [item->nsItem() setTarget:m_delegate];
+    item->nsItem().target = m_nativeMenu.delegate;
     if (!item->menu())
         [item->nsItem() setAction:@selector(itemFired:)];
 
     if (item->isMerged())
         return;
 
-    if ([item->nsItem() menu]) {
-        qWarning("Menu item is already in a menu, remove it from the other menu first before inserting");
-        return;
-    }
     // if the item we're inserting before is merged, skip along until
     // we find a non-merged real item to insert ahead of.
     while (beforeItem && beforeItem->isMerged()) {
@@ -445,12 +455,11 @@ void QCocoaMenu::setEnabled(bool enabled)
 
 bool QCocoaMenu::isEnabled() const
 {
-    return [m_nativeItem isEnabled];
+    return m_attachedItem ? [m_attachedItem isEnabled] : m_enabled;
 }
 
 void QCocoaMenu::setVisible(bool visible)
 {
-    [m_nativeItem setSubmenu:(visible ? m_nativeMenu : nil)];
     m_visible = visible;
 }
 
@@ -587,8 +596,6 @@ void QCocoaMenu::syncModalState(bool modal)
     if (!m_enabled)
         modal = true;
 
-    [m_nativeItem setEnabled:!modal];
-
     foreach (QCocoaMenuItem *item, m_menuItems) {
         if (item->menu()) { // recurse into submenus
             item->menu()->syncModalState(modal);
@@ -599,25 +606,24 @@ void QCocoaMenu::syncModalState(bool modal)
     }
 }
 
-void QCocoaMenu::setMenuBar(QCocoaMenuBar *menuBar)
+void QCocoaMenu::setAttachedItem(NSMenuItem *item)
 {
-    m_menuBar = menuBar;
-    SET_COCOA_MENU_ANCESTOR(this, menuBar);
-}
+    if (item == m_attachedItem)
+        return;
 
-QCocoaMenuBar *QCocoaMenu::menuBar() const
-{
-    return m_menuBar;
-}
+    if (m_attachedItem)
+        m_attachedItem.submenu = nil;
+
+    m_attachedItem = item;
+
+    if (m_attachedItem)
+        m_attachedItem.submenu = m_nativeMenu;
 
-void QCocoaMenu::setContainingMenuItem(QCocoaMenuItem *menuItem)
-{
-    m_containingMenuItem = menuItem;
 }
 
-QCocoaMenuItem *QCocoaMenu::containingMenuItem() const
+NSMenuItem *QCocoaMenu::attachedItem() const
 {
-    return m_containingMenuItem;
+    return m_attachedItem;
 }
 
 QT_END_NAMESPACE