QCocoaMenuItem: Make QCocoaNSMenu the item target

Since we made the NSMenu delegate a singleton we could not rely
on it to have enough information to implement worksWhenModal.
At the same time as the delegate change, we derived NSMenu into
QCocoaNSMenu. This allows us to extend the menu functionality
and, in this case, serve as target for the Cocoa menu items.

We also refactor setting the item's target/action.

Manually tested against menurama and bigmenucreator tests, the
test-case for QTBUG-17291, and the richtext/textedit example.

Change-Id: I222241f71db82611711b23d4a8c6122a741370ae
Task-number: QTBUG-66676
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

5 files changed, 54 insertions, 46 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.h b/src/plugins/platforms/cocoa/qcocoamenu.h
index 2b4cf0ef98..6db4e04c61 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.h
+++ b/src/plugins/platforms/cocoa/qcocoamenu.h
@@ -96,6 +96,8 @@ public:
 
     void syncMenuItem_helper(QPlatformMenuItem *menuItem, bool menubarUpdate);
 
+    void setItemTargetAction(QCocoaMenuItem *item) const;
+
 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 b3c2d5ae90..a788ac7d45 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -132,9 +132,8 @@ void QCocoaMenu::insertMenuItem(QPlatformMenuItem *menuItem, QPlatformMenuItem *
 
 void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
 {
+    setItemTargetAction(item);
     NSMenuItem *nativeItem = item->nsItem();
-    nativeItem.target = m_nativeMenu.delegate;
-    nativeItem.action = @selector(itemFired:);
     // Someone's adding new items after aboutToShow() was emitted
     if (isOpen() && nativeItem && item->menu())
         item->menu()->setAttachedItem(nativeItem);
@@ -496,4 +495,11 @@ NSMenuItem *QCocoaMenu::attachedItem() const
     return m_attachedItem;
 }
 
+void QCocoaMenu::setItemTargetAction(QCocoaMenuItem *item) const
+{
+    auto *nsItem = item->nsItem();
+    nsItem.target = m_nativeMenu;
+    nsItem.action = @selector(qt_itemFired:);
+}
+
 QT_END_NAMESPACE
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm
index 8091d00bda..61ac5eb7f0 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm
@@ -307,14 +307,12 @@ void QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder()
 void QCocoaMenuBar::resetKnownMenuItemsToQt()
 {
     // Undo the effect of redirectKnownMenuItemsToFirstResponder():
-    // set the menu items' actions to itemFired and their targets to
-    // the QCocoaMenuDelegate.
+    // reset the menu items' target/action.
     foreach (QCocoaMenuBar *mb, static_menubars) {
         foreach (QCocoaMenu *m, mb->m_menus) {
             foreach (QCocoaMenuItem *i, m->items()) {
                 if (i->effectiveRole() >= QPlatformMenuItem::ApplicationSpecificRole) {
-                   [i->nsItem() setTarget:m->nsMenu().delegate];
-                   [i->nsItem() setAction:@selector(itemFired:)];
+                    m->setItemTargetAction(i);
                 }
             }
         }
diff --git a/src/plugins/platforms/cocoa/qcocoansmenu.h b/src/plugins/platforms/cocoa/qcocoansmenu.h
index 8fb0a26f27..a9c3e4fff9 100644
--- a/src/plugins/platforms/cocoa/qcocoansmenu.h
+++ b/src/plugins/platforms/cocoa/qcocoansmenu.h
@@ -68,8 +68,6 @@ typedef QPointer<QCocoaMenu> QCocoaMenuPointer;
                         forKey:(NSString *)key
                      modifiers:(NSUInteger)modifiers;
 
-- (BOOL)validateMenuItem:(NSMenuItem *)item; // NSMenuValidation
-
 @end
 
 @interface QT_MANGLE_NAMESPACE(QCocoaNSMenu) : NSMenu
@@ -78,6 +76,11 @@ typedef QPointer<QCocoaMenu> QCocoaMenuPointer;
 
 - (instancetype)initWithQPAMenu:(QCocoaMenu *)menu;
 
+- (void)qt_itemFired:(NSMenuItem *)item;
+
+- (BOOL)worksWhenModal;
+- (BOOL)validateMenuItem:(NSMenuItem*)item; // NSMenuValidation
+
 @end
 
 QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaNSMenu);
diff --git a/src/plugins/platforms/cocoa/qcocoansmenu.mm b/src/plugins/platforms/cocoa/qcocoansmenu.mm
index 6be2569dbc..19a0f950e4 100644
--- a/src/plugins/platforms/cocoa/qcocoansmenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoansmenu.mm
@@ -82,6 +82,43 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
     return self;
 }
 
+// Cocoa will query the menu item's target for the worksWhenModal selector.
+// So we need to implement this to allow the items to be handled correctly
+// when a modal dialog is visible. See documentation for NSMenuItem.target.
+- (BOOL)worksWhenModal
+{
+    if (!QGuiApplication::modalWindow())
+        return YES;
+    if (const auto *mb = qobject_cast<QCocoaMenuBar *>(self.qpaMenu->menuParent()))
+        return QGuiApplication::modalWindow()->handle() == mb->cocoaWindow() ? YES : NO;
+    return YES;
+}
+
+- (void)qt_itemFired:(NSMenuItem *)item
+{
+    auto *qpaItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
+    // Menu-holding items also get a target to play nicely
+    // with NSMenuValidation but should not trigger.
+    if (!qpaItem || qpaItem->menu())
+        return;
+
+    QScopedScopeLevelCounter scopeLevelCounter(QGuiApplicationPrivate::instance()->threadData);
+    QGuiApplicationPrivate::modifier_buttons = [QNSView convertKeyModifiers:[NSEvent modifierFlags]];
+
+    static QMetaMethod activatedSignal = QMetaMethod::fromSignal(&QCocoaMenuItem::activated);
+    activatedSignal.invoke(qpaItem, Qt::QueuedConnection);
+}
+
+- (BOOL)validateMenuItem:(NSMenuItem*)item
+{
+    auto *qpaItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
+    // Menu-holding items are always enabled, as it's conventional in Cocoa
+    if (!qpaItem || qpaItem->menu())
+        return YES;
+
+    return qpaItem->isEnabled();
+}
+
 @end
 
 #define CHECK_MENU_CLASS(menu) Q_ASSERT([menu isMemberOfClass:[QCocoaNSMenu class]])
@@ -160,31 +197,6 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
     emit qpaMenu->aboutToHide();
 }
 
-- (void)itemFired:(NSMenuItem *)item
-{
-    auto *qpaItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
-    // Menu-holding items also get a target to play nicely
-    // with NSMenuValidation but should not trigger.
-    if (!qpaItem || qpaItem->menu())
-        return;
-
-    QScopedScopeLevelCounter scopeLevelCounter(QGuiApplicationPrivate::instance()->threadData);
-    QGuiApplicationPrivate::modifier_buttons = [QNSView convertKeyModifiers:[NSEvent modifierFlags]];
-
-    static QMetaMethod activatedSignal = QMetaMethod::fromSignal(&QCocoaMenuItem::activated);
-    activatedSignal.invoke(qpaItem, Qt::QueuedConnection);
-}
-
-- (BOOL)validateMenuItem:(NSMenuItem*)item
-{
-    auto *qpaItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
-    // Menu-holding items are always enabled, as it's conventional in Cocoa
-    if (!qpaItem || qpaItem->menu())
-        return YES;
-
-    return qpaItem->isEnabled();
-}
-
 - (BOOL)menuHasKeyEquivalent:(NSMenu *)menu forEvent:(NSEvent *)event target:(id *)target action:(SEL *)action
 {
     /*
@@ -293,19 +305,6 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
     return nil;
 }
 
-// Cocoa will query the menu item's target for the worksWhenModal selector.
-// So we need to implement this to allow the items to be handled correctly
-// when a modal dialog is visible.
-- (BOOL)worksWhenModal
-{
-    if (!QGuiApplication::modalWindow())
-        return YES;
-    const auto &qpaMenu = static_cast<QCocoaNSMenu *>(self).qpaMenu;
-    if (auto *mb = qobject_cast<QCocoaMenuBar *>(qpaMenu->menuParent()))
-        return QGuiApplication::modalWindow()->handle() == mb->cocoaWindow() ? YES : NO;
-    return YES;
-}
-
 @end
 
 #undef CHECK_MENU_CLASS