Get collapsible menus working correctly.

There was an attempt to do this earlier, but it was a bit more complex
than it needed to be. We now do the update on show in Cocoa. Carbon
actually does it all for us, we just need to flip the bit. We may do the
updates to often, but it's better than not enough.

Task-Id: 195445
Reviewed-by: Denis

6 files changed, 74 insertions, 54 deletions

diff --git a/src/gui/kernel/qt_cocoa_helpers_mac.mm b/src/gui/kernel/qt_cocoa_helpers_mac.mm
index 223e36b..3d4164a 100644
--- a/src/gui/kernel/qt_cocoa_helpers_mac.mm
+++ b/src/gui/kernel/qt_cocoa_helpers_mac.mm
@@ -74,6 +74,7 @@
 ****************************************************************************/
 
 #include <private/qcore_mac_p.h>
+#include <qaction.h>
 #include <qwidget.h>
 #include <qdesktopwidget.h>
 #include <qevent.h>
@@ -1193,4 +1194,32 @@ void qt_mac_constructQIconFromIconRef(const IconRef icon, const IconRef overlayI
     }
 }
 
+void qt_mac_menu_collapseSeparators(void */*NSMenu **/ theMenu, bool collapse)
+{
+    OSMenuRef menu = static_cast<OSMenuRef>(theMenu);
+    if (collapse) {
+        bool previousIsSeparator = true; // setting to true kills all the separators placed at the top.
+        NSMenuItem *previousItem = nil;
+        for (NSMenuItem *item in [menu itemArray]) {
+            if ([item isSeparatorItem]) {
+                [item setHidden:previousIsSeparator];
+            }
+
+            if (![item isHidden]) {
+                previousItem = item;
+                previousIsSeparator = ([previousItem isSeparatorItem]);
+            }
+        }
+
+        // We now need to check the final item since we don't want any separators at the end of the list.
+        if (previousItem && previousIsSeparator)
+            [previousItem setHidden:YES];
+    } else {
+        for (NSMenuItem *item in [menu itemArray]) {
+            if (QAction *action = reinterpret_cast<QAction *>([item tag]))
+                [item setHidden:!action->isVisible()];
+        }
+    }
+}
+
 QT_END_NAMESPACE
diff --git a/src/gui/kernel/qt_cocoa_helpers_mac_p.h b/src/gui/kernel/qt_cocoa_helpers_mac_p.h
index af3b4cb..2cc7dee 100644
--- a/src/gui/kernel/qt_cocoa_helpers_mac_p.h
+++ b/src/gui/kernel/qt_cocoa_helpers_mac_p.h
@@ -133,6 +133,7 @@ void qt_dispatchTabletProximityEvent(void * /*NSEvent * */ tabletEvent);
 #ifdef QT_MAC_USE_COCOA
 bool qt_dispatchKeyEventWithCocoa(void * /*NSEvent * */ keyEvent, QWidget *widgetToGetEvent);
 #endif
+void qt_mac_menu_collapseSeparators(void * /*NSMenu */ menu, bool collapse);
 bool qt_dispatchKeyEvent(void * /*NSEvent * */ keyEvent, QWidget *widgetToGetEvent);
 void qt_dispatchModifiersChanged(void * /*NSEvent * */flagsChangedEvent, QWidget *widgetToGetEvent);
 void qt_mac_dispatchNCMouseMessage(void */* NSWindow* */eventWindow, void */* NSEvent* */mouseEvent, 
diff --git a/src/gui/widgets/qcocoamenu_mac.mm b/src/gui/widgets/qcocoamenu_mac.mm
index 3338fd8..f3bb73e 100644
--- a/src/gui/widgets/qcocoamenu_mac.mm
+++ b/src/gui/widgets/qcocoamenu_mac.mm
@@ -98,7 +98,9 @@ QT_USE_NAMESPACE
     while (QWidget *popup
                 = QApplication::activePopupWidget())
         popup->close();
-    qt_mac_emit_menuSignals(((QT_MANGLE_NAMESPACE(QCocoaMenu) *)menu)->qmenu, true);
+    QMenu *qtmenu = static_cast<QT_MANGLE_NAMESPACE(QCocoaMenu) *>(menu)->qmenu;
+    qt_mac_emit_menuSignals(qtmenu, true);
+    qt_mac_menu_collapseSeparators(menu, qtmenu->separatorsCollapsible());
 }
 
 - (void)menuWillClose:(NSMenu*)menu;
diff --git a/src/gui/widgets/qmenu.cpp b/src/gui/widgets/qmenu.cpp
index 99f3880..8eec0fc 100644
--- a/src/gui/widgets/qmenu.cpp
+++ b/src/gui/widgets/qmenu.cpp
@@ -3013,12 +3013,19 @@ bool QMenu::separatorsCollapsible() const
 void QMenu::setSeparatorsCollapsible(bool collapse)
 {
     Q_D(QMenu);
+    if (d->collapsibleSeparators == collapse)
+        return;
+
     d->collapsibleSeparators = collapse;
     d->itemsDirty = 1;
     if (isVisible()) {
         d->updateActionRects();
         update();
     }
+#ifdef Q_WS_MAC
+    if (d->mac_menu)
+        d->syncSeparatorsCollapsible(collapse);
+#endif
 }
 
 #ifdef QT3_SUPPORT
diff --git a/src/gui/widgets/qmenu_mac.mm b/src/gui/widgets/qmenu_mac.mm
index 87f6f82..e6239f4 100644
--- a/src/gui/widgets/qmenu_mac.mm
+++ b/src/gui/widgets/qmenu_mac.mm
@@ -458,7 +458,8 @@ OSStatus qt_mac_menu_event(EventHandlerCallRef er, EventRef event, void *)
 
                         int merged = 0;
                         const QMenuPrivate::QMacMenuPrivate *mac_menu = qmenu->d_func()->mac_menu;
-                        for(int i = 0; i < mac_menu->actionItems.size(); ++i) {
+                        const int ActionItemsCount = mac_menu->actionItems.size();
+                        for(int i = 0; i < ActionItemsCount; ++i) {
                             QMacMenuAction *action = mac_menu->actionItems.at(i);
                             if (action->action->isSeparator()) {
                                 bool hide = false;
@@ -972,7 +973,7 @@ void Q_GUI_EXPORT qt_mac_set_menubar_merge(bool b) { qt_mac_no_menubar_merge = !
 /*****************************************************************************
   QMenu bindings
  *****************************************************************************/
-QMenuPrivate::QMacMenuPrivate::QMacMenuPrivate(QMenuPrivate *menu) : menu(0), qmenu(menu)
+QMenuPrivate::QMacMenuPrivate::QMacMenuPrivate() : menu(0)
 {
 }
 
@@ -1300,61 +1301,22 @@ QMenuPrivate::QMacMenuPrivate::syncAction(QMacMenuAction *action)
 #else
     int itemIndex = [menu indexOfItem:item];
     Q_ASSERT(itemIndex != -1);
-
-    // Separator handling: Menu items and separators can be added to a QMenu in
-    // any order (for example, add all the separators first and then "fill inn" 
-    // the menu items). Create NSMenuItem seperatorItems for the Qt separators, 
-    // and make sure that there are no double separators and no seprators 
-    // at the top or bottom of the menu.
-    bool itemIsSeparator = action->action->isSeparator();
-    bool previousItemIsSeparator = false;
-    if (itemIndex > 0) {
-        if ([[menu itemAtIndex : itemIndex - 1] isSeparatorItem])
-            previousItemIsSeparator = true;
-    }
-    bool nexItemIsSeparator = false;
-    if (itemIndex > 0 && itemIndex < [menu numberOfItems] -1) {
-        if ([[menu itemAtIndex : itemIndex + 1] isSeparatorItem])
-            nexItemIsSeparator = true;
-    }
-    bool itemIsAtBottomOfMenu = (itemIndex == [menu numberOfItems]  - 1);
-    bool itemIsAtTopOfMenu = (itemIndex == 0);
-        
-
-    if (itemIsSeparator) {
-        // Create separators items for actions that are now separators
+    if (action->action->isSeparator()) {
         action->menuItem = [NSMenuItem separatorItem];
         [action->menuItem retain];
-
-        // Hide duplicate/top/bottom separators.
-        if (qmenu->collapsibleSeparators && (previousItemIsSeparator || itemIsAtBottomOfMenu || itemIsAtTopOfMenu)) {
-            [action->menuItem setHidden : true];
-        }
-
         [menu insertItem: action->menuItem atIndex:itemIndex];
         [menu removeItem:item];
         [item release];
         item = action->menuItem;
         return;
-    } else {
-        // Create standard menu items for actions that are no longer separators
-        if ([item isSeparatorItem]) {
-            action->menuItem = createNSMenuItem(action->action->text());
-            [menu insertItem:action->menuItem atIndex:itemIndex];
-            [menu removeItem:item];
-            [item release];
-            item = action->menuItem;
-        }
-
-        // Show separators that should now be visible since a non-separator
-        // item (the current item) was added.
-        if (previousItemIsSeparator) {
-            [[menu itemAtIndex : itemIndex - 1] setHidden : false];
-        } else if (itemIsAtTopOfMenu && nexItemIsSeparator) {
-            [[menu itemAtIndex : itemIndex + 1] setHidden : false];
-        }
+    } else if ([item isSeparatorItem]) {
+        // I'm no longer a separator...
+        action->menuItem = createNSMenuItem(action->action->text());
+        [menu insertItem:action->menuItem atIndex:itemIndex];
+        [menu removeItem:item];
+        [item release];
+        item = action->menuItem;
     }
-
 #endif
 
     //find text (and accel)
@@ -1540,7 +1502,7 @@ QMenuPrivate::macMenu(OSMenuRef merge)
     if (mac_menu && mac_menu->menu)
         return mac_menu->menu;
     if (!mac_menu)
-        mac_menu = new QMacMenuPrivate(this);
+        mac_menu = new QMacMenuPrivate;
     mac_menu->menu = qt_mac_create_menu(q);
     if (merge) {
 #ifndef QT_MAC_USE_COCOA
@@ -1552,12 +1514,31 @@ QMenuPrivate::macMenu(OSMenuRef merge)
     QList<QAction*> items = q->actions();
     for(int i = 0; i < items.count(); i++)
         mac_menu->addAction(items[i], 0, this);
+    syncSeparatorsCollapsible(collapsibleSeparators);
     return mac_menu->menu;
 }
 
 /*!
   \internal
 */
+void
+QMenuPrivate::syncSeparatorsCollapsible(bool collapse)
+{
+#ifndef QT_MAC_USE_COCOA
+    if (collapse)
+        ChangeMenuAttributes(mac_menu->menu, kMenuAttrCondenseSeparators, 0);
+    else
+        ChangeMenuAttributes(mac_menu->menu, 0, kMenuAttrCondenseSeparators);
+#else
+    qt_mac_menu_collapseSeparators(mac_menu->menu, collapse);
+#endif
+}
+
+
+
+/*!
+  \internal
+*/
 void QMenuPrivate::setMacMenuEnabled(bool enable)
 {
     if (!macMenu(0))
diff --git a/src/gui/widgets/qmenu_p.h b/src/gui/widgets/qmenu_p.h
index 4e428fe..8697771 100644
--- a/src/gui/widgets/qmenu_p.h
+++ b/src/gui/widgets/qmenu_p.h
@@ -263,9 +263,8 @@ public:
     struct QMacMenuPrivate {
         QList<QMacMenuAction*> actionItems;
         OSMenuRef menu;
-        QMenuPrivate *qmenu;
-        QMacMenuPrivate(QMenuPrivate *menu);
-        ~QMacMenuPrivate();    
+        QMacMenuPrivate();
+        ~QMacMenuPrivate();
 
         bool merged(const QAction *action) const;
         void addAction(QAction *, QMacMenuAction* =0, QMenuPrivate *qmenu = 0);
@@ -285,6 +284,7 @@ public:
     } *mac_menu;
     OSMenuRef macMenu(OSMenuRef merge);
     void setMacMenuEnabled(bool enable = true);
+    void syncSeparatorsCollapsible(bool collapsible);
     static QHash<OSMenuRef, OSMenuRef> mergeMenuHash;
     static QHash<OSMenuRef, QMenuMergeList*> mergeMenuItemsHash;
 #endif