diff options
author | Gabriel de Dietrich <gabriel.dedietrich@qt.io> | 2018-04-20 11:16:17 -0700 |
---|---|---|
committer | Gabriel de Dietrich <gabriel.dedietrich@qt.io> | 2018-04-26 17:51:18 +0000 |
commit | 8412009de62ff0c9540290b6fb0b8d1f470b2cb8 (patch) | |
tree | f3146c8c153727b23161bd1693619387bc51214e /src/plugins/platforms | |
parent | 25d5c8fe5d06ab0a1161c36ddc021edc2ed76197 (diff) |
Cocoa Menus: Refactor QCocoaMenuItem::sync()
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 <morten.sorvig@qt.io>
Diffstat (limited to 'src/plugins/platforms')
-rw-r--r-- | src/plugins/platforms/cocoa/messages.cpp | 10 | ||||
-rw-r--r-- | src/plugins/platforms/cocoa/qcocoamenuitem.mm | 89 |
2 files changed, 49 insertions, 50 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 <QCoreApplication> +#include <QtCore/qcoreapplication.h> +#include <QtCore/qregularexpression.h> // 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 <QtGui/private/qcoregraphics_p.h> -#include <QtCore/qregularexpression.h> #include <QtCore/QDebug> @@ -229,74 +228,68 @@ NSMenuItem *QCocoaMenuItem::sync() } if ((m_role != NoRole && !m_textSynced) || m_merged) { - NSMenuItem *mergeItem = nil; + 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; + while (depth < 3 && p && !(menubar = qobject_cast<QCocoaMenuBar *>(p))) { + ++depth; + QCocoaMenuObject *menuObject = dynamic_cast<QCocoaMenuObject *>(p); + Q_ASSERT(menuObject); + p = menuObject->menuParent(); + } + + if (menubar && depth < 3) + m_detectedRole = detectMenuRole(m_text); + else + m_detectedRole = NoRole; + } + QCocoaMenuLoader *loader = [QCocoaMenuLoader sharedMenuLoader]; - switch (m_role) { - case ApplicationSpecificRole: - mergeItem = [loader appSpecificMenuItem:this]; - break; + 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 PreferencesRole: - mergeItem = [loader preferencesMenuItem]; + case CutRole: + case CopyRole: + case PasteRole: + case SelectAllRole: + if (menubar) + mergeItem = menubar->itemForRole(role); break; - case TextHeuristicRole: { - QObject *p = menuParent(); - int depth = 1; - QCocoaMenuBar *menubar = nullptr; - while (depth < 3 && p && !(menubar = qobject_cast<QCocoaMenuBar *>(p))) { - ++depth; - QCocoaMenuObject *menuObject = dynamic_cast<QCocoaMenuObject *>(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; - } + 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<QCocoaNSMenuItem *>(mergeItem).platformMenuItem = this; [m_native release]; m_native = mergeItem; + if (auto *nativeItem = qt_objc_cast<QCocoaNSMenuItem *>(m_native)) + nativeItem.platformMenuItem = this; } else if (m_merged) { // was previously merged, but no longer [m_native release]; |