From 664b368ace67b3c7a1bb45a023cf1a911239716a Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Wed, 30 Jun 2021 11:14:31 +0200
Subject: [PATCH] macOS: fix separator actions not showing in menus
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Amends a3f3af8a8fb8f8b09d1685df5ab836244f850a62, which resulted in the
last visible separator always getting hidden.

Don't abuse QCocoaMenuItem::visible property to store whether the native
NSMenuItem is shown, only store whether it should be shown.

Rename the local variables to simplify the logic.

Fixes: QTBUG-94802
Change-Id: I56e8c99a2a46d8cbdf75c4ad6cb714961f28a6a3
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 6832789bbd1a19499bffd05b1cac400db2bdd948)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/plugins/platforms/cocoa/qcocoamenu.mm | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index e22f619a6ef..d6af2a55234 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -299,31 +299,26 @@ void QCocoaMenu::syncSeparatorsCollapsible(bool enable)
     QMacAutoReleasePool pool;
     if (enable) {
         bool previousIsSeparator = true; // setting to true kills all the separators placed at the top.
-        NSMenuItem *previousItem = nil;
+        NSMenuItem *lastVisibleItem = nil;
 
         for (NSMenuItem *item in m_nativeMenu.itemArray) {
             if (item.separatorItem) {
                 // hide item if previous was a separator, or if it's explicitly hidden
-                bool itemVisible = !previousIsSeparator;
-                if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem) {
-                    cocoaItem->setVisible(!previousIsSeparator && cocoaItem->isVisible());
-                    itemVisible = cocoaItem->isVisible();
-                }
-                item.hidden = !itemVisible;
+                bool hideItem = previousIsSeparator;
+                if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(item).platformMenuItem)
+                    hideItem = previousIsSeparator || !cocoaItem->isVisible();
+                item.hidden = hideItem;
             }
 
             if (!item.hidden) {
-                previousItem = item;
-                previousIsSeparator = previousItem.separatorItem;
+                lastVisibleItem = item;
+                previousIsSeparator = lastVisibleItem.separatorItem;
             }
         }
 
         // We now need to check the final item since we don't want any separators at the end of the list.
-        if (previousItem && previousIsSeparator) {
-            if (auto *cocoaItem = qt_objc_cast<QCocoaNSMenuItem *>(previousItem).platformMenuItem)
-                cocoaItem->setVisible(false);
-            previousItem.hidden = YES;
-        }
+        if (lastVisibleItem && lastVisibleItem.separatorItem)
+            lastVisibleItem.hidden = YES;
     } else {
         for (auto *item : qAsConst(m_menuItems)) {
             if (!item->isSeparator())