From 8412009de62ff0c9540290b6fb0b8d1f470b2cb8 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Fri, 20 Apr 2018 11:16:17 -0700 Subject: [PATCH] Cocoa Menus: Refactor QCocoaMenuItem::sync() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit And move some logic into detectMenuRole(), where it belongs. This refactoring will enable fixes for the issues below. Change-Id: Id03bb5c26d7dd0bb3b94f01e69935e1f3321bb95 Task-number: QTBUG-17291 Task-number: QTBUG-30812 Task-number: QTBUG-38705 Reviewed-by: Morten Johan Sørvig --- src/plugins/platforms/cocoa/messages.cpp | 10 +- src/plugins/platforms/cocoa/qcocoamenuitem.mm | 91 +++++++++---------- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/src/plugins/platforms/cocoa/messages.cpp b/src/plugins/platforms/cocoa/messages.cpp index c7e7f82bae..06e3dd454e 100644 --- a/src/plugins/platforms/cocoa/messages.cpp +++ b/src/plugins/platforms/cocoa/messages.cpp @@ -39,7 +39,8 @@ #include "messages.h" -#include +#include +#include // Translatable messages should go into this .cpp file for them to be picked up by lupdate. @@ -77,8 +78,13 @@ QPlatformMenuItem::MenuRole detectMenuRole(const QString &caption) QString captionNoAmpersand(caption); captionNoAmpersand.remove(QLatin1Char('&')); const QString aboutString = QCoreApplication::translate("QCocoaMenuItem", "About"); - if (captionNoAmpersand.startsWith(aboutString, Qt::CaseInsensitive) || caption.endsWith(aboutString, Qt::CaseInsensitive)) + if (captionNoAmpersand.startsWith(aboutString, Qt::CaseInsensitive) + || captionNoAmpersand.endsWith(aboutString, Qt::CaseInsensitive)) { + static const QRegularExpression qtRegExp(QLatin1String("qt$"), QRegularExpression::CaseInsensitiveOption); + if (captionNoAmpersand.contains(qtRegExp)) + return QPlatformMenuItem::AboutQtRole; return QPlatformMenuItem::AboutRole; + } if (captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Config"), Qt::CaseInsensitive) || captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Preference"), Qt::CaseInsensitive) || captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Options"), Qt::CaseInsensitive) diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.mm b/src/plugins/platforms/cocoa/qcocoamenuitem.mm index 1011d97685..e1572e1800 100644 --- a/src/plugins/platforms/cocoa/qcocoamenuitem.mm +++ b/src/plugins/platforms/cocoa/qcocoamenuitem.mm @@ -51,7 +51,6 @@ #include "qcocoaapplication.h" // for custom application category #include "qcocoamenuloader.h" #include -#include #include @@ -229,74 +228,68 @@ NSMenuItem *QCocoaMenuItem::sync() } if ((m_role != NoRole && !m_textSynced) || m_merged) { - NSMenuItem *mergeItem = nil; - QCocoaMenuLoader *loader = [QCocoaMenuLoader sharedMenuLoader]; - switch (m_role) { - case ApplicationSpecificRole: - mergeItem = [loader appSpecificMenuItem:this]; - break; - case AboutRole: - mergeItem = [loader aboutMenuItem]; - break; - case AboutQtRole: - mergeItem = [loader aboutQtMenuItem]; - break; - case QuitRole: - mergeItem = [loader quitMenuItem]; - break; - case PreferencesRole: - mergeItem = [loader preferencesMenuItem]; - break; - case TextHeuristicRole: { + QCocoaMenuBar *menubar = nullptr; + if (m_role == TextHeuristicRole) { + // Recognized menu roles are only found in the first menus below the menubar QObject *p = menuParent(); int depth = 1; - QCocoaMenuBar *menubar = nullptr; while (depth < 3 && p && !(menubar = qobject_cast(p))) { ++depth; QCocoaMenuObject *menuObject = dynamic_cast(p); Q_ASSERT(menuObject); p = menuObject->menuParent(); } - if (depth == 3 || !menubar) - break; // Menu item too deep in the hierarchy, or not connected to any menubar - m_detectedRole = detectMenuRole(m_text); - switch (m_detectedRole) { - case QPlatformMenuItem::AboutRole: - if (m_text.indexOf(QRegularExpression(QString::fromLatin1("qt$"), - QRegularExpression::CaseInsensitiveOption)) == -1) - mergeItem = [loader aboutMenuItem]; - else - mergeItem = [loader aboutQtMenuItem]; - break; - case QPlatformMenuItem::PreferencesRole: - mergeItem = [loader preferencesMenuItem]; - break; - case QPlatformMenuItem::QuitRole: - mergeItem = [loader quitMenuItem]; - break; - default: - if (m_detectedRole >= CutRole && m_detectedRole < RoleCount && menubar) - mergeItem = menubar->itemForRole(m_detectedRole); - if (!m_text.isEmpty()) - m_textSynced = true; - break; - } - break; + if (menubar && depth < 3) + m_detectedRole = detectMenuRole(m_text); + else + m_detectedRole = NoRole; } + QCocoaMenuLoader *loader = [QCocoaMenuLoader sharedMenuLoader]; + NSMenuItem *mergeItem = nil; + const auto role = effectiveRole(); + switch (role) { + case AboutRole: + mergeItem = [loader aboutMenuItem]; + break; + case AboutQtRole: + mergeItem = [loader aboutQtMenuItem]; + break; + case PreferencesRole: + mergeItem = [loader preferencesMenuItem]; + break; + case ApplicationSpecificRole: + mergeItem = [loader appSpecificMenuItem:this]; + break; + case QuitRole: + mergeItem = [loader quitMenuItem]; + break; + case CutRole: + case CopyRole: + case PasteRole: + case SelectAllRole: + if (menubar) + mergeItem = menubar->itemForRole(role); + break; + case NoRole: + // The heuristic couldn't resolve the menu role + m_textSynced = false; + break; default: - qWarning() << "Menu item" << m_text << "has unsupported role" << m_role; + if (!m_text.isEmpty()) + m_textSynced = true; + break; } if (mergeItem) { m_textSynced = true; m_merged = true; [mergeItem retain]; - if ([mergeItem isMemberOfClass:[QCocoaNSMenuItem class]]) - static_cast(mergeItem).platformMenuItem = this; [m_native release]; m_native = mergeItem; + if (auto *nativeItem = qt_objc_cast(m_native)) + nativeItem.platformMenuItem = this; } else if (m_merged) { // was previously merged, but no longer [m_native release];