Cocoa Menus: Use the responder chain for menu items target/action

We start by setting the menu item target to nil.

Then, -[qt_itemFired:] action is now in QNSView, which itself is
naturally inserted in the responder chain. This removes the need
to track and change the menu item's target/action when we're
displaying a native dialog. Part of this is possible because we
now derive our own QCocoaNSMenuItem class from NSMenuItem.

We use -[respondsToSelector:] to decide whether the QNSView in
the responder chain should respond to cut:, copy:, etc. And we
only return YES when the view is first responder. The invocation
to these action is forwarded to the same views' -[qt_itemFired:].

Message forwarding is done via forwardInvocation:, but experiments
have shown that it can be done by the sole means of respondsToSelector:
and direct invocation from cut:, copy:, etc. See the usage of the
macro QT_COCOA_DYNAMIC_MENU_ITEM_ACTION.

Menu validation also happens in QNSView and looks for modal windows.
Therefore, -[worksWhenModal] is no longer necessary. Also, since the
target is no longer set, the logic as documented in NSMenuItem.target
won't work anymore.

Most items from QCocoaMenuLoader also become QCocoaNSMenuItem and
get the same target/action, which removes a bit of duplicated (and
outdated) code. A particular case is the Quit item, which gets the
terminate: action set until an actual menu item is added.

Tested with texedit and standard dialogs examples together with
menus, menurama and bigmenucreator manual tests.

We also renamed some functions and variables to reflect common
naming practices.

Change-Id: I9b51d3be3467a666d8c3dcf8585edbc821e0282e
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

13 files changed, 280 insertions, 223 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm
index 55c4e51242..63af002c3c 100644
--- a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm
+++ b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm
@@ -50,7 +50,6 @@
 #include <private/qguiapplication_p.h>
 #include "qt_mac_p.h"
 #include "qcocoahelpers.h"
-#include "qcocoamenubar.h"
 #include "qcocoaeventdispatcher.h"
 #include <qregexp.h>
 #include <qbuffer.h>
@@ -220,7 +219,6 @@ static QString strippedText(QString s)
             || [self panel:nil shouldEnableURL:url];
 
         [self updateProperties];
-        QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder();
         [mSavePanel setNameFieldStringValue:selectable ? info.fileName().toNSString() : @""];
 
         [mOpenPanel beginWithCompletionHandler:^(NSInteger result){
@@ -250,9 +248,7 @@ static QString strippedText(QString s)
     // Make sure we don't interrupt the runModal call below.
     QCocoaEventDispatcher::clearCurrentThreadCocoaEventDispatcherInterruptFlag();
 
-    QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder();
     mReturnCode = [mSavePanel runModal];
-    QCocoaMenuBar::resetKnownMenuItemsToQt();
 
     QAbstractEventDispatcher::instance()->interrupt();
     return (mReturnCode == NSModalResponseOK);
@@ -272,7 +268,6 @@ static QString strippedText(QString s)
         || [self panel:nil shouldEnableURL:url];
 
     [self updateProperties];
-    QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder();
     [mSavePanel setDirectoryURL: [NSURL fileURLWithPath:mCurrentDir]];
 
     [mSavePanel setNameFieldStringValue:selectable ? info.fileName().toNSString() : @""];
@@ -594,7 +589,6 @@ void QCocoaFileDialogHelper::QNSOpenSavePanelDelegate_selectionChanged(const QSt
 
 void QCocoaFileDialogHelper::QNSOpenSavePanelDelegate_panelClosed(bool accepted)
 {
-    QCocoaMenuBar::resetKnownMenuItemsToQt();
     if (accepted) {
         emit accept();
     } else {
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 49b3e76606..75d5ea2dd8 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -62,7 +62,7 @@ QCocoaMenu::QCocoaMenu() :
 {
     QMacAutoReleasePool pool;
 
-    m_nativeMenu = [[QCocoaNSMenu alloc] initWithQPAMenu:this];
+    m_nativeMenu = [[QCocoaNSMenu alloc] initWithPlatformMenu:this];
 }
 
 QCocoaMenu::~QCocoaMenu()
@@ -132,7 +132,7 @@ void QCocoaMenu::insertMenuItem(QPlatformMenuItem *menuItem, QPlatformMenuItem *
 
 void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
 {
-    setItemTargetAction(item);
+    item->resolveTargetAction();
     NSMenuItem *nativeItem = item->nsItem();
     // Someone's adding new items after aboutToShow() was emitted
     if (isOpen() && nativeItem && item->menu())
@@ -423,7 +423,7 @@ QPlatformMenuItem *QCocoaMenu::menuItemAt(int position) const
     if (0 <= position && position < m_menuItems.count())
         return m_menuItems.at(position);
 
-    return 0;
+    return nullptr;
 }
 
 QPlatformMenuItem *QCocoaMenu::menuItemForTag(quintptr tag) const
@@ -433,7 +433,7 @@ QPlatformMenuItem *QCocoaMenu::menuItemForTag(quintptr tag) const
             return item;
     }
 
-    return 0;
+    return nullptr;
 }
 
 QList<QCocoaMenuItem *> QCocoaMenu::items() const
@@ -493,11 +493,4 @@ 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.h b/src/plugins/platforms/cocoa/qcocoamenubar.h
index 6f3aca3a51..fbf6c49eeb 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.h
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.h
@@ -65,8 +65,6 @@ public:
     inline NSMenu *nsMenu() const
         { return m_nativeMenu; }
 
-    static void redirectKnownMenuItemsToFirstResponder();
-    static void resetKnownMenuItemsToQt();
     static void updateMenuBarImmediately();
 
     QList<QCocoaMenuItem*> merged() const;
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm
index 61ac5eb7f0..1b6b5a5de6 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm
@@ -86,7 +86,6 @@ QCocoaMenuBar::~QCocoaMenuBar()
         // the menu bar was updated
         qDeleteAll(children());
         updateMenuBarImmediately();
-        resetKnownMenuItemsToQt();
     }
 }
 
@@ -259,66 +258,6 @@ QCocoaMenuBar *QCocoaMenuBar::findGlobalMenubar()
     return NULL;
 }
 
-void QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder()
-{
-    // QTBUG-17291: http://forums.macrumors.com/showthread.php?t=1249452
-    // When a dialog is opened, shortcuts for actions inside the dialog (cut, paste, ...)
-    // continue to go through the same menu items which claimed those shortcuts.
-    // They are not keystrokes which we can intercept in any other way; the OS intercepts them.
-    // The menu items had to be created by the application.  That's why we need roles
-    // to identify those "special" menu items which can be useful even when non-Qt
-    // native widgets are in focus.  When the native widget is focused it will be the
-    // first responder, so the menu item needs to have its target be the first responder;
-    // this is done by setting it to nil.
-
-    // This function will find all menu items on all menus which have
-    // "special" roles, set the target and also set the standard actions which
-    // apply to those roles.  But afterwards it is necessary to call
-    // resetKnownMenuItemsToQt() to put back the target and action so that
-    // those menu items will go back to invoking their associated QActions.
-    foreach (QCocoaMenuBar *mb, static_menubars)
-        foreach (QCocoaMenu *m, mb->m_menus)
-            foreach (QCocoaMenuItem *i, m->items()) {
-                bool known = true;
-                switch (i->effectiveRole()) {
-                case QPlatformMenuItem::CutRole:
-                    [i->nsItem() setAction:@selector(cut:)];
-                    break;
-                case QPlatformMenuItem::CopyRole:
-                    [i->nsItem() setAction:@selector(copy:)];
-                    break;
-                case QPlatformMenuItem::PasteRole:
-                    [i->nsItem() setAction:@selector(paste:)];
-                    break;
-                case QPlatformMenuItem::SelectAllRole:
-                    [i->nsItem() setAction:@selector(selectAll:)];
-                    break;
-                // We may discover later that there are other roles/actions which
-                // are meaningful to standard native widgets; they can be added.
-                default:
-                    known = false;
-                    break;
-                }
-                if (known)
-                    [i->nsItem() setTarget:nil];
-            }
-}
-
-void QCocoaMenuBar::resetKnownMenuItemsToQt()
-{
-    // Undo the effect of redirectKnownMenuItemsToFirstResponder():
-    // 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) {
-                    m->setItemTargetAction(i);
-                }
-            }
-        }
-    }
-}
-
 void QCocoaMenuBar::updateMenuBarImmediately()
 {
     QMacAutoReleasePool pool;
@@ -438,7 +377,7 @@ QPlatformMenu *QCocoaMenuBar::menuForTag(quintptr tag) const
             return menu;
     }
 
-    return 0;
+    return nullptr;
 }
 
 NSMenuItem *QCocoaMenuBar::itemForRole(QPlatformMenuItem::MenuRole r)
diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.h b/src/plugins/platforms/cocoa/qcocoamenuitem.h
index 2b598ee3a0..9c4a308b7a 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuitem.h
+++ b/src/plugins/platforms/cocoa/qcocoamenuitem.h
@@ -108,6 +108,7 @@ public:
 
     QCocoaMenu *menu() const { return m_menu; }
     MenuRole effectiveRole() const;
+    void resolveTargetAction();
 
 private:
     QString mergeText();
diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.mm b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
index 59e0150168..bd8cab9b23 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuitem.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
@@ -41,6 +41,7 @@
 
 #include "qcocoamenuitem.h"
 
+#include "qcocoansmenu.h"
 #include "qcocoamenu.h"
 #include "qcocoamenubar.h"
 #include "messages.h"
@@ -232,7 +233,7 @@ NSMenuItem *QCocoaMenuItem::sync()
         QCocoaMenuLoader *loader = [QCocoaMenuLoader sharedMenuLoader];
         switch (m_role) {
         case ApplicationSpecificRole:
-            mergeItem = [loader appSpecificMenuItem:reinterpret_cast<NSInteger>(this)];
+            mergeItem = [loader appSpecificMenuItem:this];
             break;
         case AboutRole:
             mergeItem = [loader aboutMenuItem];
@@ -249,7 +250,7 @@ NSMenuItem *QCocoaMenuItem::sync()
         case TextHeuristicRole: {
             QObject *p = menuParent();
             int depth = 1;
-            QCocoaMenuBar *menubar = 0;
+            QCocoaMenuBar *menubar = nullptr;
             while (depth < 3 && p && !(menubar = qobject_cast<QCocoaMenuBar *>(p))) {
                 ++depth;
                 QCocoaMenuObject *menuObject = dynamic_cast<QCocoaMenuObject *>(p);
@@ -292,9 +293,10 @@ NSMenuItem *QCocoaMenuItem::sync()
             m_textSynced = true;
             m_merged = true;
             [mergeItem retain];
+            if ([mergeItem isMemberOfClass:[QCocoaNSMenuItem class]])
+                static_cast<QCocoaNSMenuItem *>(mergeItem).platformMenuItem = this;
             [m_native release];
             m_native = mergeItem;
-            [m_native setTag:reinterpret_cast<NSInteger>(this)];
         } else if (m_merged) {
             // was previously merged, but no longer
             [m_native release];
@@ -306,12 +308,12 @@ NSMenuItem *QCocoaMenuItem::sync()
     }
 
     if (!m_native) {
-        m_native = [[NSMenuItem alloc] initWithTitle:m_text.toNSString()
-                                       action:nil
-                                       keyEquivalent:@""];
-        [m_native setTag:reinterpret_cast<NSInteger>(this)];
+        m_native = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:this];
+        m_native.title = m_text.toNSString();
     }
 
+    resolveTargetAction();
+
     [m_native setHidden: !m_isVisible];
     [m_native setView:m_itemView];
 
@@ -431,4 +433,36 @@ void QCocoaMenuItem::setIconSize(int size)
     m_iconSize = size;
 }
 
+void QCocoaMenuItem::resolveTargetAction()
+{
+    // Some items created by QCocoaMenuLoader are not
+    // instances of QCocoaNSMenuItem and have their
+    // target/action set as Interface Builder would.
+    if (![m_native isMemberOfClass:[QCocoaNSMenuItem class]])
+        return;
+
+    // Use the responder chain and ensure native modal dialogs
+    // continue receiving cut/copy/paste/etc. key equivalents.
+    SEL roleAction;
+    switch (effectiveRole()) {
+    case CutRole:
+        roleAction = @selector(cut:);
+        break;
+    case CopyRole:
+        roleAction = @selector(copy:);
+        break;
+    case PasteRole:
+        roleAction = @selector(paste:);
+        break;
+    case SelectAllRole:
+        roleAction = @selector(selectAll:);
+        break;
+    default:
+        roleAction = @selector(qt_itemFired:);
+    }
+
+    m_native.action = roleAction;
+    m_native.target = nil;
+}
+
 QT_END_NAMESPACE
diff --git a/src/plugins/platforms/cocoa/qcocoamenuloader.h b/src/plugins/platforms/cocoa/qcocoamenuloader.h
index 2fe669b489..f743dc69a1 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuloader.h
+++ b/src/plugins/platforms/cocoa/qcocoamenuloader.h
@@ -54,17 +54,18 @@
 #import <AppKit/AppKit.h>
 #include <QtCore/private/qcore_mac_p.h>
 
-@interface QT_MANGLE_NAMESPACE(QCocoaMenuLoader) : NSResponder
+QT_FORWARD_DECLARE_CLASS(QCocoaMenuItem);
+
+@interface QT_MANGLE_NAMESPACE(QCocoaMenuLoader) : NSObject
 + (instancetype)sharedMenuLoader;
+- (NSMenu *)menu;
 - (void)ensureAppMenuInMenu:(NSMenu *)menu;
-- (void)removeActionsFromAppMenu;
 - (NSMenuItem *)quitMenuItem;
 - (NSMenuItem *)preferencesMenuItem;
 - (NSMenuItem *)aboutMenuItem;
 - (NSMenuItem *)aboutQtMenuItem;
 - (NSMenuItem *)hideMenuItem;
-- (NSMenuItem *)appSpecificMenuItem:(NSInteger)tag;
-- (void)qtDispatcherToQPAMenuItem:(id)sender;
+- (NSMenuItem *)appSpecificMenuItem:(QCocoaMenuItem *)platformItem;
 - (void)qtTranslateApplicationMenu;
 - (NSArray<NSMenuItem *> *)mergeable;
 @end
diff --git a/src/plugins/platforms/cocoa/qcocoamenuloader.mm b/src/plugins/platforms/cocoa/qcocoamenuloader.mm
index d4d7bc1d7a..00776f4b18 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuloader.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuloader.mm
@@ -41,6 +41,7 @@
 
 #include "messages.h"
 #include "qcocoahelpers.h"
+#include "qcocoansmenu.h"
 #include "qcocoamenubar.h"
 #include "qcocoamenuitem.h"
 #include "qcocoaintegration.h"
@@ -95,17 +96,19 @@
         appItem.submenu = appMenu;
 
         // About Application
-        aboutItem = [[NSMenuItem alloc] initWithTitle:[@"About " stringByAppendingString:appName]
-                                               action:@selector(orderFrontStandardAboutPanel:)
-                                        keyEquivalent:@""];
+        aboutItem = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:nullptr];
+        aboutItem.title = [@"About " stringByAppendingString:appName];
+        // FIXME This seems useless since barely adding a QAction
+        // with AboutRole role will reset the target/action
         aboutItem.target = self;
+        aboutItem.action = @selector(orderFrontStandardAboutPanel:);
         // Disable until a QAction is associated
         aboutItem.enabled = NO;
         aboutItem.hidden = YES;
         [appMenu addItem:aboutItem];
 
         // About Qt (shameless self-promotion)
-        aboutQtItem = [[NSMenuItem alloc] init];
+        aboutQtItem = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:nullptr];
         aboutQtItem.title = @"About Qt";
         // Disable until a QAction is associated
         aboutQtItem.enabled = NO;
@@ -115,10 +118,9 @@
         [appMenu addItem:[NSMenuItem separatorItem]];
 
         // Preferences
-        preferencesItem = [[NSMenuItem alloc] initWithTitle:@"Preferences…"
-                                                     action:@selector(qtDispatcherToQPAMenuItem:)
-                                              keyEquivalent:@","];
-        preferencesItem.target = self;
+        preferencesItem = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:nullptr];
+        preferencesItem.title = @"Preferences…";
+        preferencesItem.keyEquivalent = @",";
         // Disable until a QAction is associated
         preferencesItem.enabled = NO;
         preferencesItem.hidden = YES;
@@ -161,10 +163,13 @@
         [appMenu addItem:[NSMenuItem separatorItem]];
 
         // Quit Application
-        quitItem = [[NSMenuItem alloc] initWithTitle:[@"Quit " stringByAppendingString:appName]
-                                              action:@selector(terminate:)
-                                       keyEquivalent:@"q"];
-        quitItem.target = self;
+        quitItem = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:nullptr];
+        quitItem.title = [@"Quit " stringByAppendingString:appName];
+        quitItem.keyEquivalent = @"q";
+        // This will remain true until synced with a QCocoaMenuItem.
+        // This way, we will always have a functional Quit menu item
+        // even if no QAction is added.
+        quitItem.action = @selector(terminate:);
         [appMenu addItem:quitItem];
     }
 
@@ -225,12 +230,6 @@
     }
 }
 
-- (void)removeActionsFromAppMenu
-{
-    for (NSMenuItem *item in [appMenu itemArray])
-        [item setTag:0];
-}
-
 - (NSMenu *)menu
 {
     return [[theMenu retain] autorelease];
@@ -266,17 +265,18 @@
     return [[hideItem retain] autorelease];
 }
 
-- (NSMenuItem *)appSpecificMenuItem:(NSInteger)tag
+- (NSMenuItem *)appSpecificMenuItem:(QCocoaMenuItem *)platformItem
 {
-    NSMenuItem *item = [appMenu itemWithTag:tag];
-
-    // No reason to create the item if it already exists. See QTBUG-27202.
-    if (item)
-        return [[item retain] autorelease];
+    for (NSMenuItem *item in appMenu.itemArray)
+        if ([item isMemberOfClass:[QCocoaNSMenuItem class]]
+            && static_cast<QCocoaNSMenuItem *>(item).platformMenuItem == platformItem) {
+            // No reason to create the item if it already exists.
+            return [[item retain] autorelease];
+        }
 
     // Create an App-Specific menu item, insert it into the menu and return
     // it as an autorelease item.
-    item = [[NSMenuItem alloc] init];
+    QCocoaNSMenuItem *item = [[QCocoaNSMenuItem alloc] initWithPlatformMenuItem:platformItem];
 
     NSInteger location;
     if (lastAppSpecificItem == nil) {
@@ -291,16 +291,6 @@
     return [[item retain] autorelease];
 }
 
-- (BOOL) acceptsFirstResponder
-{
-    return YES;
-}
-
-- (void)terminate:(id)sender
-{
-    [NSApp terminate:sender];
-}
-
 - (void)orderFrontStandardAboutPanel:(id)sender
 {
     [NSApp orderFrontStandardAboutPanel:sender];
@@ -335,44 +325,19 @@
 #endif
 }
 
-- (void)qtDispatcherToQPAMenuItem:(id)sender
-{
-    NSMenuItem *item = static_cast<NSMenuItem *>(sender);
-    if (item == quitItem) {
-        // We got here because someone was once the quitItem, but it has been
-        // abandoned (e.g., the menubar was deleted). In the meantime, just do
-        // normal QApplication::quit().
-        qApp->quit();
-        return;
-    }
-
-    if ([item tag]) {
-        QCocoaMenuItem *cocoaItem = reinterpret_cast<QCocoaMenuItem *>([item tag]);
-        QScopedScopeLevelCounter scopeLevelCounter(QGuiApplicationPrivate::instance()->threadData);
-        cocoaItem->activated();
-    }
-}
-
-- (void)orderFrontCharacterPalette:(id)sender
-{
-    [NSApp orderFrontCharacterPalette:sender];
-}
-
 - (BOOL)validateMenuItem:(NSMenuItem*)menuItem
 {
-    if ([menuItem action] == @selector(hideOtherApplications:)
-        || [menuItem action] == @selector(unhideAllApplications:)) {
+    if (menuItem.action == @selector(hideOtherApplications:)
+        || menuItem.action == @selector(unhideAllApplications:))
         return [NSApp validateMenuItem:menuItem];
-    } else if ([menuItem action] == @selector(hide:)) {
+
+    if (menuItem.action == @selector(hide:)) {
         if (QCocoaIntegration::instance()->activePopupWindow())
             return NO;
         return [NSApp validateMenuItem:menuItem];
-    } else if ([menuItem tag]) {
-        QCocoaMenuItem *cocoaItem = reinterpret_cast<QCocoaMenuItem *>([menuItem tag]);
-        return cocoaItem->isEnabled();
-    } else {
-        return [menuItem isEnabled];
     }
+
+    return menuItem.enabled;
 }
 
 - (NSArray<NSMenuItem *> *)mergeable
diff --git a/src/plugins/platforms/cocoa/qcocoansmenu.h b/src/plugins/platforms/cocoa/qcocoansmenu.h
index a9c3e4fff9..6988c2a084 100644
--- a/src/plugins/platforms/cocoa/qcocoansmenu.h
+++ b/src/plugins/platforms/cocoa/qcocoansmenu.h
@@ -53,12 +53,10 @@
 
 #import <AppKit/AppKit.h>
 
-#include <QtCore/qpointer.h>
 #include <qcocoahelpers.h>
 
 QT_FORWARD_DECLARE_CLASS(QCocoaMenu);
-typedef QPointer<QCocoaMenu> QCocoaMenuPointer;
-
+QT_FORWARD_DECLARE_CLASS(QCocoaMenuItem);
 
 @interface QT_MANGLE_NAMESPACE(QCocoaNSMenuDelegate) : NSObject <NSMenuDelegate>
 
@@ -72,18 +70,22 @@ typedef QPointer<QCocoaMenu> QCocoaMenuPointer;
 
 @interface QT_MANGLE_NAMESPACE(QCocoaNSMenu) : NSMenu
 
-@property (readonly, nonatomic) QCocoaMenuPointer qpaMenu;
+@property (readonly, nonatomic) QCocoaMenu *platformMenu;
+
+- (instancetype)initWithPlatformMenu:(QCocoaMenu *)menu;
+
+@end
 
-- (instancetype)initWithQPAMenu:(QCocoaMenu *)menu;
+@interface QT_MANGLE_NAMESPACE(QCocoaNSMenuItem) : NSMenuItem
 
-- (void)qt_itemFired:(NSMenuItem *)item;
+@property (nonatomic) QCocoaMenuItem *platformMenuItem;
 
-- (BOOL)worksWhenModal;
-- (BOOL)validateMenuItem:(NSMenuItem*)item; // NSMenuValidation
+- (instancetype)initWithPlatformMenuItem:(QCocoaMenuItem *)menuItem;
 
 @end
 
 QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaNSMenu);
+QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaNSMenuItem);
 QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaNSMenuDelegate);
 
 #endif // QCOCOANSMENU_H
diff --git a/src/plugins/platforms/cocoa/qcocoansmenu.mm b/src/plugins/platforms/cocoa/qcocoansmenu.mm
index 19a0f950e4..99c56f9191 100644
--- a/src/plugins/platforms/cocoa/qcocoansmenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoansmenu.mm
@@ -70,11 +70,14 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
 }
 
 @implementation QCocoaNSMenu
+{
+    QPointer<QCocoaMenu> _platformMenu;
+}
 
-- (instancetype)initWithQPAMenu:(QCocoaMenu *)menu
+- (instancetype)initWithPlatformMenu:(QCocoaMenu *)menu
 {
     if ((self = [super initWithTitle:@"Untitled"])) {
-        _qpaMenu = menu;
+        _platformMenu = menu;
         self.autoenablesItems = YES;
         self.delegate = [QCocoaNSMenuDelegate sharedMenuDelegate];
     }
@@ -82,41 +85,35 @@ 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
+- (QCocoaMenu *)platformMenu
 {
-    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;
+    return _platformMenu.data();
 }
 
-- (void)qt_itemFired:(NSMenuItem *)item
+@end
+
+@implementation QCocoaNSMenuItem
 {
-    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;
+    QPointer<QCocoaMenuItem> _platformMenuItem;
+}
 
-    QScopedScopeLevelCounter scopeLevelCounter(QGuiApplicationPrivate::instance()->threadData);
-    QGuiApplicationPrivate::modifier_buttons = [QNSView convertKeyModifiers:[NSEvent modifierFlags]];
+- (instancetype)initWithPlatformMenuItem:(QCocoaMenuItem *)menuItem
+{
+    if ((self = [super initWithTitle:@"" action:nil keyEquivalent:@""])) {
+        _platformMenuItem = menuItem;
+    }
 
-    static QMetaMethod activatedSignal = QMetaMethod::fromSignal(&QCocoaMenuItem::activated);
-    activatedSignal.invoke(qpaItem, Qt::QueuedConnection);
+    return self;
 }
 
-- (BOOL)validateMenuItem:(NSMenuItem*)item
+- (QCocoaMenuItem *)platformMenuItem
 {
-    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 _platformMenuItem.data();
+}
 
-    return qpaItem->isEnabled();
+- (void)setPlatformMenuItem:(QCocoaMenuItem *)menuItem
+{
+    _platformMenuItem = menuItem;
 }
 
 @end
@@ -153,14 +150,16 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
     if (shouldCancel)
         return NO;
 
-    const auto &qpaMenu = static_cast<QCocoaNSMenu *>(menu).qpaMenu;
-    if (qpaMenu.isNull())
+    const auto &platformMenu = static_cast<QCocoaNSMenu *>(menu).platformMenu;
+    if (!platformMenu)
         return YES;
 
-    auto *menuItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
-    if (qpaMenu->items().contains(menuItem)) {
-        if (QCocoaMenu *itemSubmenu = menuItem->menu())
-            itemSubmenu->setAttachedItem(item);
+    if ([item isMemberOfClass:[QCocoaNSMenuItem class]]) {
+        auto *menuItem = static_cast<QCocoaNSMenuItem *>(item).platformMenuItem;
+        if (platformMenu->items().contains(menuItem)) {
+            if (QCocoaMenu *itemSubmenu = menuItem->menu())
+                itemSubmenu->setAttachedItem(item);
+        }
     }
 
     return YES;
@@ -169,32 +168,33 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
 - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
 {
     CHECK_MENU_CLASS(menu);
-    auto *qpaItem = reinterpret_cast<QCocoaMenuItem *>(item.tag);
-    if (qpaItem)
-        qpaItem->hovered();
+    if ([item isMemberOfClass:[QCocoaNSMenuItem class]]) {
+        if (auto *platformItem = static_cast<QCocoaNSMenuItem *>(item).platformMenuItem)
+            emit platformItem->hovered();
+    }
 }
 
 - (void)menuWillOpen:(NSMenu *)menu
 {
     CHECK_MENU_CLASS(menu);
-    const auto &qpaMenu = static_cast<QCocoaNSMenu *>(menu).qpaMenu;
-    if (qpaMenu.isNull())
+    auto *platformMenu = static_cast<QCocoaNSMenu *>(menu).platformMenu;
+    if (!platformMenu)
         return;
 
-    qpaMenu->setIsOpen(true);
-    emit qpaMenu->aboutToShow();
+    platformMenu->setIsOpen(true);
+    emit platformMenu->aboutToShow();
 }
 
 - (void)menuDidClose:(NSMenu *)menu
 {
     CHECK_MENU_CLASS(menu);
-    const auto &qpaMenu = static_cast<QCocoaNSMenu *>(menu).qpaMenu;
-    if (qpaMenu.isNull())
+    auto *platformMenu = static_cast<QCocoaNSMenu *>(menu).platformMenu;
+    if (!platformMenu)
         return;
 
-    qpaMenu->setIsOpen(false);
+    platformMenu->setIsOpen(false);
     // wrong, but it's the best we can do
-    emit qpaMenu->aboutToHide();
+    emit platformMenu->aboutToHide();
 }
 
 - (BOOL)menuHasKeyEquivalent:(NSMenu *)menu forEvent:(NSEvent *)event target:(id *)target action:(SEL *)action
@@ -229,19 +229,6 @@ static NSString *qt_mac_removePrivateUnicode(NSString* string)
     }
 
     if (keyEquivalentItem) {
-        if (!keyEquivalentItem.target) {
-            // This item was modified by QCocoaMenuBar::redirectKnownMenuItemsToFirstResponder
-            // and it looks like we're running a modal session for NSOpenPanel/NSSavePanel.
-            // QCocoaFileDialogHelper is actually the only place we use this and we run NSOpenPanel modal
-            // (modal sheet, window modal, application modal).
-            // Whatever the current first responder is, let's give it a chance
-            // and do not touch the Qt's focusObject (which is different from some native view
-            // having a focus inside NSSave/OpenPanel.
-            *target = nil;
-            *action = keyEquivalentItem.action;
-            return YES;
-        }
-
         QObject *object = qApp->focusObject();
         if (object) {
             QChar ch;
diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h
index ba696f4b5f..f7ca41d3be 100644
--- a/src/plugins/platforms/cocoa/qnsview.h
+++ b/src/plugins/platforms/cocoa/qnsview.h
@@ -52,6 +52,7 @@ class QPointF;
 QT_END_NAMESPACE
 
 Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper));
+Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QCocoaNSMenuItem));
 
 @interface QT_MANGLE_NAMESPACE(QNSView) : NSView
 
diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index 8fc6482f36..ff3ab8aae7 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -361,6 +361,7 @@
 #include "qnsview_dragging.mm"
 #include "qnsview_keys.mm"
 #include "qnsview_complextext.mm"
+#include "qnsview_menus.mm"
 
 // -----------------------------------------------------
 
diff --git a/src/plugins/platforms/cocoa/qnsview_menus.mm b/src/plugins/platforms/cocoa/qnsview_menus.mm
new file mode 100644
index 0000000000..15c14a1236
--- /dev/null
+++ b/src/plugins/platforms/cocoa/qnsview_menus.mm
@@ -0,0 +1,141 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the plugins of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+// This file is included from qnsview.mm, and only used to organize the code
+
+#include <qcocoansmenu.h>
+#include <qcocoamenuitem.h>
+#include <qcocoamenu.h>
+#include <qcocoamenubar.h>
+
+static bool selectorIsCutCopyPaste(SEL selector)
+{
+    return (selector == @selector(cut:)
+            || selector == @selector(copy:)
+            || selector == @selector(paste:)
+            || selector == @selector(selectAll:));
+}
+
+@interface QT_MANGLE_NAMESPACE(QNSView) (Menus)
+- (void)qt_itemFired:(QCocoaNSMenuItem *)item;
+@end
+
+@implementation QT_MANGLE_NAMESPACE(QNSView) (Menus)
+
+- (BOOL)validateMenuItem:(NSMenuItem*)item
+{
+    if (![item isMemberOfClass:[QCocoaNSMenuItem class]])
+        return item.enabled; // FIXME Test with with Qt as plugin or embedded QWindow.
+
+    auto *platformItem = static_cast<QCocoaNSMenuItem *>(item).platformMenuItem;
+    if (!platformItem)
+        return NO;
+
+    // Menu-holding items are always enabled, as it's conventional in Cocoa
+    if (platformItem->menu())
+        return YES;
+
+    // Check if a modal dialog is active. Validate only menu
+    // items belonging to this view's window own menu bar.
+    if (QGuiApplication::modalWindow()) {
+        QCocoaMenuBar *menubar = nullptr;
+
+        QObject *menuParent = platformItem->menuParent();
+        while (menuParent && !(menubar = qobject_cast<QCocoaMenuBar *>(menuParent))) {
+            auto *menuObject = dynamic_cast<QCocoaMenuObject *>(menuParent);
+            menuParent = menuObject->menuParent();
+        }
+
+        if (menubar && menubar->cocoaWindow() != self.platformWindow)
+            return NO;
+    }
+
+    return platformItem->isEnabled();
+}
+
+- (BOOL)respondsToSelector:(SEL)selector
+{
+    // Not exactly true. Both copy: and selectAll: can work on non key views.
+    if (selectorIsCutCopyPaste(selector))
+        return ([NSApp keyWindow] == self.window) && (self.window.firstResponder == self);
+
+    return [super respondsToSelector:selector];
+}
+
+- (void)qt_itemFired:(QCocoaNSMenuItem *)item
+{
+    Q_ASSERT([item isMemberOfClass:[QCocoaNSMenuItem class]]);
+    auto *platformItem = static_cast<QCocoaNSMenuItem *>(item).platformMenuItem;
+    // Menu-holding items also get a target to play nicely
+    // with NSMenuValidation but should not trigger.
+    if (!platformItem || platformItem->menu())
+        return;
+
+    QScopedScopeLevelCounter scopeLevelCounter(QGuiApplicationPrivate::instance()->threadData);
+    QGuiApplicationPrivate::modifier_buttons = [QNSView convertKeyModifiers:[NSEvent modifierFlags]];
+
+    static QMetaMethod activatedSignal = QMetaMethod::fromSignal(&QCocoaMenuItem::activated);
+    activatedSignal.invoke(platformItem, Qt::QueuedConnection);
+}
+
+- (NSMethodSignature *)methodSignatureForSelector:(SEL)selector
+{
+    if (selectorIsCutCopyPaste(selector)) {
+        NSMethodSignature *itemFiredSign = [super methodSignatureForSelector:@selector(qt_itemFired:)];
+        return itemFiredSign;
+    }
+
+    return [super methodSignatureForSelector:selector];
+}
+
+- (void)forwardInvocation:(NSInvocation *)invocation
+{
+    if (selectorIsCutCopyPaste(invocation.selector)) {
+        NSObject *sender;
+        [invocation getArgument:&sender atIndex:2];
+        if ([sender isMemberOfClass:[QCocoaNSMenuItem class]]) {
+            [self qt_itemFired:static_cast<QCocoaNSMenuItem *>(sender)];
+            return;
+        }
+    }
+
+    [super forwardInvocation:invocation];
+}
+
+@end