QCocoaMenu: Attach menu items when updating the menubar

Instead of waiting for the menu delegate to update each item,
we can attach an NSMenu to its NSMenuItem as soon as we update
the current window's menubar. This is safe to do because we
know that this is going to be the main menubar right after, so
we're not orphaning any NSMenuItem from its NSMenu at the wrong
moment.

By doing this, we also ensure that all menus from the active
menubar are reachable by the key-equivalent dispatching logic,
even before we display the actual menu.

This was shown in BigMenuCreator where, under the menubar's ASP
and SAP menus, all A*S submenus would be disabled. Furthermore,
on the same menus, SAP would show the same issue.

Added test in Menurama as well.

Change-Id: If6e7311072e6b53ad1cbced73623d1832aa0df8e
Task-number: QTBUG-57076
Task-number: QTBUG-63712
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>

4 files changed, 27 insertions, 4 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.h b/src/plugins/platforms/cocoa/qcocoamenu.h
index 06688dbf3d..7baaf971f4 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.h
+++ b/src/plugins/platforms/cocoa/qcocoamenu.h
@@ -98,6 +98,8 @@ public:
 
     void timerEvent(QTimerEvent *e) Q_DECL_OVERRIDE;
 
+    void syncMenuItem_helper(QPlatformMenuItem *menuItem, bool menubarUpdate);
+
 private:
     QCocoaMenuItem *itemOrNull(int index) const;
     void insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem);
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 3a11023a4d..8bdd0512de 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -435,6 +435,11 @@ void QCocoaMenu::timerEvent(QTimerEvent *e)
 
 void QCocoaMenu::syncMenuItem(QPlatformMenuItem *menuItem)
 {
+    syncMenuItem_helper(menuItem, false /*menubarUpdate*/);
+}
+
+void QCocoaMenu::syncMenuItem_helper(QPlatformMenuItem *menuItem, bool menubarUpdate)
+{
     QMacAutoReleasePool pool;
     QCocoaMenuItem *cocoaItem = static_cast<QCocoaMenuItem *>(menuItem);
     if (!m_menuItems.contains(cocoaItem)) {
@@ -444,8 +449,9 @@ void QCocoaMenu::syncMenuItem(QPlatformMenuItem *menuItem)
 
     const bool wasMerged = cocoaItem->isMerged();
     NSMenuItem *oldItem = cocoaItem->nsItem();
+    NSMenuItem *syncedItem = cocoaItem->sync();
 
-    if (cocoaItem->sync() != oldItem) {
+    if (syncedItem != oldItem) {
         // native item was changed for some reason
         if (oldItem) {
             if (wasMerged) {
@@ -463,6 +469,14 @@ void QCocoaMenu::syncMenuItem(QPlatformMenuItem *menuItem)
         // when an item's enabled state changes after menuWillOpen:
         scheduleUpdate();
     }
+
+    // This may be a good moment to attach this item's eventual submenu to the
+    // synced item, but only on the condition we're all currently hooked to the
+    // menunbar. A good indicator of this being the right moment is knowing that
+    // we got called from QCocoaMenuBar::updateMenuBarImmediately().
+    if (menubarUpdate)
+        if (QCocoaMenu *submenu = cocoaItem->menu())
+            submenu->setAttachedItem(syncedItem);
 }
 
 void QCocoaMenu::syncSeparatorsCollapsible(bool enable)
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.h b/src/plugins/platforms/cocoa/qcocoamenubar.h
index 0725e9db68..a4ee531e91 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.h
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.h
@@ -72,6 +72,8 @@ public:
     QList<QCocoaMenuItem*> merged() const;
     NSMenuItem *itemForRole(QPlatformMenuItem::MenuRole r);
 
+    void syncMenu_helper(QPlatformMenu *menu, bool menubarUpdate);
+
 private:
     static QCocoaWindow *findWindowForMenubar();
     static QCocoaMenuBar *findGlobalMenubar();
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm
index 3e466c9587..a4cd465dae 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm
@@ -155,7 +155,7 @@ void QCocoaMenuBar::insertMenu(QPlatformMenu *platformMenu, QPlatformMenu *befor
         }
     }
 
-    syncMenu(menu);
+    syncMenu_helper(menu, false /*internaCall*/);
 
     if (needsImmediateUpdate())
         updateMenuBarImmediately();
@@ -183,11 +183,16 @@ void QCocoaMenuBar::removeMenu(QPlatformMenu *platformMenu)
 
 void QCocoaMenuBar::syncMenu(QPlatformMenu *menu)
 {
+    syncMenu_helper(menu, false /*internaCall*/);
+}
+
+void QCocoaMenuBar::syncMenu_helper(QPlatformMenu *menu, bool menubarUpdate)
+{
     QMacAutoReleasePool pool;
 
     QCocoaMenu *cocoaMenu = static_cast<QCocoaMenu *>(menu);
     Q_FOREACH (QCocoaMenuItem *item, cocoaMenu->items())
-        cocoaMenu->syncMenuItem(item);
+        cocoaMenu->syncMenuItem_helper(item, menubarUpdate);
 
     BOOL shouldHide = YES;
     if (cocoaMenu->isVisible()) {
@@ -357,7 +362,7 @@ void QCocoaMenuBar::updateMenuBarImmediately()
         menu->setAttachedItem(item);
         menu->setMenuParent(mb);
         // force a sync?
-        mb->syncMenu(menu);
+        mb->syncMenu_helper(menu, true /*menubarUpdate*/);
         menu->propagateEnabledState(!disableForModal);
     }