[Libreoffice-commits] core.git: Branch 'libreoffice-4-2-5' - framework/source vcl/source

[Andrzej Hunt]

 framework/source/uielement/toolbarmanager.cxx |   16 ++++++++++++++--
 vcl/source/window/toolbox2.cxx                |    8 +++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

New commits:
commit f8187a37122c7847fe80d2ba687b67e36ba60d4a
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date:   Sun Jun 1 08:40:43 2014 +0100

    fdo79368 Don't process overflow menu items twice.
    
    We reuse the toolbox overflow menu for toolbarmanagers context
    menu -- toolbarmanager previously added its menu listener to the
    toolboxes menu permanently, meaning that it would try to handle
    overflow menu items (in addition to the context menu items which
    it should handle), instead we should only add the listener when
    we are actually using the menu as a context menu.
    
    Perhaps it would be better in the long run to actually use fully separate
    menus instead, and ask toolbox to specifically add its items to that
    rather than trying to hack the context menu on top of the overflow menu?
    
    Change-Id: Iecface2c6eae9ab79dbcdb25ffdbaf446e2885ea
    (cherry picked from commit 1ae89d189200c1e351a396a3f02612b84a69985f)
    Reviewed-on: https://gerrit.libreoffice.org/9603
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/framework/source/uielement/toolbarmanager.cxx b/framework/source/uielement/toolbarmanager.cxx
index 4137de3..98e0446 100644
--- a/framework/source/uielement/toolbarmanager.cxx
+++ b/framework/source/uielement/toolbarmanager.cxx
@@ -233,8 +233,6 @@ ToolBarManager::ToolBarManager( const Reference< XComponentContext >& rxContext,
     m_pToolBar->SetCommandHdl( LINK( this, ToolBarManager, Command ) );
     m_pToolBar->SetMenuType( nMenuType );
     m_pToolBar->SetMenuButtonHdl( LINK( this, ToolBarManager, MenuButton ) );
-    m_pToolBar->GetMenu()->SetSelectHdl( LINK( this, ToolBarManager, MenuSelect ) );
-    m_pToolBar->GetMenu()->SetDeactivateHdl( LINK( this, ToolBarManager, MenuDeactivate ) );
 
     // set name for testtool, the useful part is after the last '/'
     sal_Int32 idx = rResourceName.lastIndexOf('/');
@@ -1795,10 +1793,24 @@ IMPL_LINK( ToolBarManager, Command, CommandEvent*, pCmdEvt )
     ::PopupMenu * pMenu = GetToolBarCustomMenu(m_pToolBar);
     if (pMenu)
     {
+        // We only want to handle events for the context menu, but not events
+        // on the toolbars overflow menu, hence we should only receive events
+        // from the toolbox menu when we are actually showing it as our context
+        // menu (the same menu retrieved with  GetMenu() is reused for both the
+        // overflow and context menus). If we set these Hdls permanently rather
+        // than just when the context menu is showing, then events are duplicated
+        // when the menu is being used as an overflow menu.
+        m_pToolBar->GetMenu()->SetSelectHdl( LINK( this, ToolBarManager, MenuSelect ) );
+        m_pToolBar->GetMenu()->SetDeactivateHdl( LINK( this, ToolBarManager, MenuDeactivate ) );
+
         // make sure all disabled entries will be shown
         pMenu->SetMenuFlags( pMenu->GetMenuFlags() | MENU_FLAG_ALWAYSSHOWDISABLEDENTRIES );
         ::Point aPoint( pCmdEvt->GetMousePosPixel() );
         pMenu->Execute( m_pToolBar, aPoint );
+
+        // Unlink our listeners again -- see above for why.
+        m_pToolBar->GetMenu()->SetSelectHdl( Link() );
+        m_pToolBar->GetMenu()->SetDeactivateHdl( Link() );
     }
 
     return 0;
diff --git a/vcl/source/window/toolbox2.cxx b/vcl/source/window/toolbox2.cxx
index 608b27c..2fc6b4b 100644
--- a/vcl/source/window/toolbox2.cxx
+++ b/vcl/source/window/toolbox2.cxx
@@ -2184,7 +2184,13 @@ void ToolBox::ImplExecuteCustomMenu()
             // call button handler to allow for menu customization
             mpData->maMenuButtonHdl.Call( this );
 
-        // register handler
+        // We specifically only register this event listener when executing our
+        // overflow menu (and remove it directly afterwards), as the same menu
+        // is reused for both the overflow menu (as managed here in ToolBox),
+        // but also by ToolBarManager for its context menu. If we leave event
+        // listeners alive beyond when the menu is showing in the desired mode
+        // then duplicate events can happen as the context menu "duplicates"
+        // items from the overflow menu, which both listeners would then act on.
         GetMenu()->AddEventListener( LINK( this, ToolBox, ImplCustomMenuListener ) );
 
         // make sure all disabled entries will be shown