Cocoa QPA Menus: Propagate enabled state downwards

NSMenu has autoenableItems set to true by default, and
we keep it this way in Qt. This means that NSMenuItem's
enabled property is basically ignored and therefore
QCocoaMenuItem::syncModalState() is wrong.

What is also wrong, is syncModalState()'s name in both
QCocoaMenuItem and QCocoaMenu. Indeed, this function's
role should be to ensure that the enabled state is
properly propagated down the menu hierarchy, whether
the reason is being in the context of a modal dialog
or the parent menu having been disabled by the app.
Notice that the latter case is specially needed when
a menubar menu is explicitly disabled.

Therefore, we introduce a separate flag for the parent
enabled state in order to avoid polluting the app-set
enabled state flag. This is done in both QCocoaMenu
and QCocoaMenuItem.

In the case of QCocoaMenuItem, these two flags define
whether an NSMenuItem is enabled state conjointly, and
set from -[QCocoaMenuDelegate validateMenuItem:]. The
rest of the logic remains as before. Similar logic is
used in QCocoaMenu::isEnabled().

In addition, the presence of the second flag allows us
to show disabled submenus in the same fashion native
Cocoa applications do. This means, the submenu item
itself remains enabled, allowing to show the submenu
popup where all its menu items will appear disabled.

Bonus change: merged all the bool flags into a bitfield
and made the compiler happy about the ivar reordering
in QCocoaMenu and QCocoaMenuItem's constructor.

Task-number: QTBUG-54698
Task-number: QTBUG-55121
Change-Id: Ie156cb3aa57a519103908ad4605f7b43c57e5aef
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@theqtcompany.com>

1 files changed, 23 insertions, 14 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 155abcc7ea..2be6c0335b 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -154,10 +154,10 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QCocoaMenuDelegate);
 
 - (BOOL)validateMenuItem:(NSMenuItem*)menuItem
 {
-    if (![menuItem tag])
+    QCocoaMenuItem *cocoaItem = reinterpret_cast<QCocoaMenuItem *>(menuItem.tag);
+    if (!cocoaItem)
         return YES;
 
-    QCocoaMenuItem* cocoaItem = reinterpret_cast<QCocoaMenuItem *>([menuItem tag]);
     return cocoaItem->isEnabled();
 }
 
@@ -255,6 +255,7 @@ QCocoaMenu::QCocoaMenu() :
     m_attachedItem(0),
     m_tag(0),
     m_enabled(true),
+    m_parentEnabled(true),
     m_visible(true),
     m_isOpen(false)
 {
@@ -330,6 +331,8 @@ void QCocoaMenu::insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem)
     else if (isOpen() && item->nsItem()) // Someone's adding new items after aboutToShow() was emitted
         item->menu()->setAttachedItem(item->nsItem());
 
+    item->setParentEnabled(isEnabled());
+
     if (item->isMerged())
         return;
 
@@ -374,6 +377,9 @@ void QCocoaMenu::removeMenuItem(QPlatformMenuItem *menuItem)
     if (cocoaItem->menuParent() == this)
         cocoaItem->setMenuParent(0);
 
+    // Ignore any parent enabled state
+    cocoaItem->setParentEnabled(true);
+
     m_menuItems.removeOne(cocoaItem);
     if (!cocoaItem->isMerged()) {
         if (m_nativeMenu != [cocoaItem->nsItem() menu]) {
@@ -464,13 +470,17 @@ void QCocoaMenu::syncSeparatorsCollapsible(bool enable)
 
 void QCocoaMenu::setEnabled(bool enabled)
 {
+    if (m_enabled == enabled)
+        return;
     m_enabled = enabled;
-    syncModalState(!m_enabled);
+    const bool wasParentEnabled = m_parentEnabled;
+    propagateEnabledState(m_enabled);
+    m_parentEnabled = wasParentEnabled; // Reset to the parent value
 }
 
 bool QCocoaMenu::isEnabled() const
 {
-    return m_attachedItem ? [m_attachedItem isEnabled] : m_enabled;
+    return m_attachedItem ? [m_attachedItem isEnabled] : m_enabled && m_parentEnabled;
 }
 
 void QCocoaMenu::setVisible(bool visible)
@@ -604,20 +614,19 @@ QList<QCocoaMenuItem *> QCocoaMenu::merged() const
     return result;
 }
 
-void QCocoaMenu::syncModalState(bool modal)
+void QCocoaMenu::propagateEnabledState(bool enabled)
 {
-    QMacAutoReleasePool pool;
+    QMacAutoReleasePool pool; // FIXME Is this still needed for Creator? See 6a0bb4206a2928b83648
 
-    if (!m_enabled)
-        modal = true;
+    m_parentEnabled = enabled;
+    if (!m_enabled && enabled) // Some ancestor was enabled, but this menu is not
+        return;
 
     foreach (QCocoaMenuItem *item, m_menuItems) {
-        if (item->menu()) { // recurse into submenus
-            item->menu()->syncModalState(modal);
-            continue;
-        }
-
-        item->syncModalState(modal);
+        if (QCocoaMenu *menu = item->menu())
+            menu->propagateEnabledState(enabled);
+        else
+            item->setParentEnabled(enabled);
     }
 }