From e16c6dfb7286264ccdfd37d8e74c97f13ece7835 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Tue, 27 Feb 2018 10:43:48 -0800 Subject: [PATCH] QCocoaMenuItem: Make QCocoaNSMenu the item target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ø --- src/plugins/platforms/cocoa/qcocoamenu.h | 2 + src/plugins/platforms/cocoa/qcocoamenu.mm | 10 ++- src/plugins/platforms/cocoa/qcocoamenubar.mm | 6 +- src/plugins/platforms/cocoa/qcocoansmenu.h | 7 +- src/plugins/platforms/cocoa/qcocoansmenu.mm | 75 ++++++++++---------- 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 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 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(self.qpaMenu->menuParent())) + return QGuiApplication::modalWindow()->handle() == mb->cocoaWindow() ? YES : NO; + return YES; +} + +- (void)qt_itemFired:(NSMenuItem *)item +{ + auto *qpaItem = reinterpret_cast(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(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(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(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(self).qpaMenu; - if (auto *mb = qobject_cast(qpaMenu->menuParent())) - return QGuiApplication::modalWindow()->handle() == mb->cocoaWindow() ? YES : NO; - return YES; -} - @end #undef CHECK_MENU_CLASS