macOS: Detect menu bar roles for untranslated menu titles as well

If an application added an "About" menu action, and then loaded
the qtbase translations, we would try to match the incoming menu
title ("About") against QCoreApplication::translate("QCocoaMenuItem",
"About"), which since qtbase provides translation in the QCocoaMenuItem
context would fail, and we would not detect the menu item role
successfully.

For this to work, the application developer would need to add their
own translations for every menu item, and these translations would
have to match the QCocoaMenuItem translations in qtbase.

By also comparing the menu titles against the untranslated heuristics
we now account for the situation that the app has not translated
its menu items.

If the app does add translations of its menu items these still need
to match the QCocoaMenuItem context translations.

Change-Id: Ic2f019cd42b7e080187f9738840f84b0cec239df
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 71 insertions, 36 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.mm b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
index 213326f892..0acae8d679 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuitem.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
@@ -180,38 +180,71 @@ void QCocoaMenuItem::setNativeContents(WId item)
     m_itemView.needsDisplay = YES;
 }
 
-static QPlatformMenuItem::MenuRole detectMenuRole(const QString &caption)
+static QPlatformMenuItem::MenuRole detectMenuRole(const QString &captionWithPossibleMnemonic)
 {
-    QString captionNoAmpersand(caption);
-    captionNoAmpersand.remove(u'&');
-    const QString aboutString = QCoreApplication::translate("QCocoaMenuItem", "About");
-    if (captionNoAmpersand.startsWith(aboutString, Qt::CaseInsensitive)
-        || captionNoAmpersand.endsWith(aboutString, Qt::CaseInsensitive)) {
+    QString itemCaption(captionWithPossibleMnemonic);
+    itemCaption.remove(u'&');
+
+    static const std::tuple<QPlatformMenuItem::MenuRole, std::vector<std::tuple<Qt::MatchFlags, const char *>>> roleMap[] = {
+        { QPlatformMenuItem::AboutRole, {
+            { Qt::MatchStartsWith | Qt::MatchEndsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "About") }
+        }},
+        { QPlatformMenuItem::PreferencesRole, {
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Config") },
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Preference") },
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Options") },
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Setting") },
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Setup") },
+        }},
+        { QPlatformMenuItem::QuitRole, {
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Quit") },
+            { Qt::MatchStartsWith, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Exit") },
+        }},
+        { QPlatformMenuItem::CutRole, {
+            { Qt::MatchExactly, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Cut") }
+        }},
+        { QPlatformMenuItem::CopyRole, {
+            { Qt::MatchExactly, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Copy") }
+        }},
+        { QPlatformMenuItem::PasteRole, {
+            { Qt::MatchExactly, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Paste") }
+        }},
+        { QPlatformMenuItem::SelectAllRole, {
+            { Qt::MatchExactly, QT_TRANSLATE_NOOP("QCocoaMenuItem", "Select All") }
+        }},
+    };
+
+    auto match = [](const QString &caption, const QString &itemCaption, Qt::MatchFlags matchFlags) {
+        if (matchFlags.testFlag(Qt::MatchExactly))
+            return !itemCaption.compare(caption, Qt::CaseInsensitive);
+        if (matchFlags.testFlag(Qt::MatchStartsWith) && itemCaption.startsWith(caption, Qt::CaseInsensitive))
+            return true;
+        if (matchFlags.testFlag(Qt::MatchEndsWith) && itemCaption.endsWith(caption, Qt::CaseInsensitive))
+            return true;
+        return false;
+    };
+
+    QPlatformMenuItem::MenuRole detectedRole = [&]{
+        for (const auto &[role, captions] : roleMap) {
+            for (const auto &[matchFlags, caption] : captions) {
+                // Check for untranslated match
+                if (match(caption, itemCaption, matchFlags))
+                    return role;
+                // Then translated with the current Qt translation
+                if (match(QCoreApplication::translate("QCocoaMenuItem", caption), itemCaption, matchFlags))
+                    return role;
+            }
+        }
+        return QPlatformMenuItem::NoRole;
+    }();
+
+    if (detectedRole == QPlatformMenuItem::AboutRole) {
         static const QRegularExpression qtRegExp("qt$"_L1, QRegularExpression::CaseInsensitiveOption);
-        if (captionNoAmpersand.contains(qtRegExp))
-            return QPlatformMenuItem::AboutQtRole;
-        return QPlatformMenuItem::AboutRole;
+        if (itemCaption.contains(qtRegExp))
+            detectedRole = QPlatformMenuItem::AboutQtRole;
     }
-    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)
-        || captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Setting"), Qt::CaseInsensitive)
-        || captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Setup"), Qt::CaseInsensitive)) {
-        return QPlatformMenuItem::PreferencesRole;
-    }
-    if (captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Quit"), Qt::CaseInsensitive)
-        || captionNoAmpersand.startsWith(QCoreApplication::translate("QCocoaMenuItem", "Exit"), Qt::CaseInsensitive)) {
-        return QPlatformMenuItem::QuitRole;
-    }
-    if (!captionNoAmpersand.compare(QCoreApplication::translate("QCocoaMenuItem", "Cut"), Qt::CaseInsensitive))
-        return QPlatformMenuItem::CutRole;
-    if (!captionNoAmpersand.compare(QCoreApplication::translate("QCocoaMenuItem", "Copy"), Qt::CaseInsensitive))
-        return QPlatformMenuItem::CopyRole;
-    if (!captionNoAmpersand.compare(QCoreApplication::translate("QCocoaMenuItem", "Paste"), Qt::CaseInsensitive))
-        return QPlatformMenuItem::PasteRole;
-    if (!captionNoAmpersand.compare(QCoreApplication::translate("QCocoaMenuItem", "Select All"), Qt::CaseInsensitive))
-        return QPlatformMenuItem::SelectAllRole;
-    return QPlatformMenuItem::NoRole;
+
+    return detectedRole;
 }
 
 NSMenuItem *QCocoaMenuItem::sync()
diff --git a/src/widgets/widgets/qmenubar.cpp b/src/widgets/widgets/qmenubar.cpp
index a53bc97516..a5c2c894a1 100644
--- a/src/widgets/widgets/qmenubar.cpp
+++ b/src/widgets/widgets/qmenubar.cpp
@@ -596,11 +596,13 @@ void QMenuBar::initStyleOption(QStyleOptionMenuItem *option, const QAction *acti
 
     Qt for \macos also provides a menu bar merging feature to make
     QMenuBar conform more closely to accepted \macos menu bar layout.
-    The merging functionality is based on string matching the title of
-    a QMenu entry. These strings are translated (using QObject::tr())
-    in the "QMenuBar" context. If an entry is moved its slots will still
-    fire as if it was in the original place. The table below outlines
-    the strings looked for and where the entry is placed if matched:
+    If an entry is moved its slots will still fire as if it was in the
+    original place.
+
+    The merging functionality is based on the QAction::menuRole() of
+    the menu entries. If an item has QAction::TextHeuristicRole,
+    the role is determined by string matching the title using the
+    following heuristics:
 
     \table
     \header \li String matches \li Placement \li Notes
@@ -618,8 +620,8 @@ void QMenuBar::initStyleOption(QStyleOptionMenuItem *option, const QAction *acti
             created to call QCoreApplication::quit()
     \endtable
 
-    You can override this behavior by using the QAction::menuRole()
-    property.
+    You can override this behavior by setting the QAction::menuRole()
+    property to QAction::NoRole.
 
     If you want all windows in a Mac application to share one menu
     bar, you must create a menu bar that does not have a parent.