Toolbar overflow/context menu: keep ordering and don't discard separator.

Previously the overflow/context menu should have had a separator shown
between the overflow items and the hidden items, however these were
inadvertently removed by ImplClearPopupMenu which filters the items
before the menu is displayed.

The previous ordering of items was also the REVERSE of the ordering in
the toolbar -- the overflow menu is a logical extension of the toolbar
hence items should be in the same order as they were in the toolbar.

(cherry picked from commit 3dc6808532d86c4b00a6cb81e0adb74878c13fdd)

Conflicts:
	vcl/source/window/toolbox2.cxx

Change-Id: I8444f4814fea64be1d8f8790445ad6aa01532e70
Reviewed-on: https://gerrit.libreoffice.org/9171
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

1 files changed, 13 insertions, 10 deletions

diff --git a/vcl/source/window/toolbox2.cxx b/vcl/source/window/toolbox2.cxx
index bf7c05fc52b6..608b27cd8c27 100644
--- a/vcl/source/window/toolbox2.cxx
+++ b/vcl/source/window/toolbox2.cxx
@@ -2119,39 +2119,42 @@ void ToolBox::UpdateCustomMenu()
             i++;
     }
 
-    // add menu items, starting from the end and inserting at pos 0
+    // add menu items: first the overflow items, then hidden items, both in the
+    // order they would usually appear in the toolbar. Separators that would be
+    // in the toolbar are ignored as they would introduce too much clutter,
+    // instead we have a single separator to help distinguish between overflow
+    // and hidden items.
     if ( !mpData->m_aItems.empty() )
     {
         // nStartPos will hold the number of clipped items appended from first loop
-        sal_uInt16 nSepPos = 0;
-        for ( std::vector< ImplToolItem >::reverse_iterator it(mpData->m_aItems.rbegin());
-                it != mpData->m_aItems.rend(); ++it)
+        for ( std::vector< ImplToolItem >::iterator it(mpData->m_aItems.begin());
+                it != mpData->m_aItems.end(); ++it)
         {
             if( it->IsClipped() )
             {
                 sal_uInt16 id = it->mnId + TOOLBOX_MENUITEM_START;
-                pMenu->InsertItem( id, it->maText, it->maImage, 0, OString(), 0 );
+                pMenu->InsertItem( id, it->maText, it->maImage, 0, OString());
                 pMenu->EnableItem( id, it->mbEnabled );
                 pMenu->CheckItem ( id, it->meState == STATE_CHECK );
-                nSepPos++;
             }
         }
 
         // add a separator below the inserted clipped-items
-        pMenu->InsertSeparator( OString(), nSepPos );
+        pMenu->InsertSeparator();
 
         // now append the items that are explicitly disabled
-        for ( std::vector< ImplToolItem >::reverse_iterator it(mpData->m_aItems.rbegin());
-                it != mpData->m_aItems.rend(); ++it)
+        for ( std::vector< ImplToolItem >::iterator it(mpData->m_aItems.begin());
+                it != mpData->m_aItems.end(); ++it)
         {
             if( it->IsItemHidden() )
             {
                 sal_uInt16 id = it->mnId + TOOLBOX_MENUITEM_START;
-                pMenu->InsertItem( id, it->maText, it->maImage, 0, OString(), nSepPos+1 );
+                pMenu->InsertItem( id, it->maText, it->maImage, 0, OString() );
                 pMenu->EnableItem( id, it->mbEnabled );
                 pMenu->CheckItem( id, it->meState == STATE_CHECK );
             }
         }
+
     }
 }