[Libreoffice-commits] core.git: framework/inc framework/source

Maxim Monastirsky (via logerrit) logerrit at kemper.freedesktop.org
Sun Sep 6 22:19:28 UTC 2020


 framework/inc/menuconfiguration.hxx           |    2 
 framework/inc/uielement/menubarmanager.hxx    |    9 -
 framework/source/uielement/menubarmanager.cxx |  130 ++------------------------
 3 files changed, 12 insertions(+), 129 deletions(-)

New commits:
commit 34f37ce6b9feef6091e0f27a2618e3f812f42008
Author:     Maxim Monastirsky <momonasmon at gmail.com>
AuthorDate: Tue Sep 1 23:25:41 2020 +0300
Commit:     Maxim Monastirsky <momonasmon at gmail.com>
CommitDate: Mon Sep 7 00:18:43 2020 +0200

    MenuBarManager: Simplify addon menu creation
    
    Addon menus were created with a different ctor, which called
    Init instead of FillMenuManager. But the former is just a
    subset of the latter, and I don't see in the latter anything
    particularly harmful for addon menus.
    
    There was however the bool _bHandlePopUp parameter, which
    controlled whether to create popup menus to be used by popup
    menu controllers, but: (a) it doesn't make any sense to me to
    allow controllers in some addon menus but not in others, and
    (b) the Activate method creates controllers unconditionally,
    which means that a controller might still be created, but
    then get nullptr for the popup menu, and crash.
    
    There was also m_bIsBookmarkMenu, which was set to true for
    addon menus, and used in the Select method to add Referer
    argument to the executed command. As a matter of fact, this
    argument is useless, as the referer is never evaluated for any
    command or macro execution. Only affected case might be when
    content URLs used directly as menu commands. But such usage
    isn't common, and even then an empty referer is similarly
    accepted by SvtSecurityOptions::isUntrustedReferer. However
    seeing the message of f0a9ca24fd4bf79cac908bf0d6fdb8905dc504db
    ("rhbz#887420 Implement "block untrusted referer links" feature")
    it appears that it's better to have the explicit referer anyway
    (but with a different check, as m_bIsBookmarkMenu is now gone).
    
    (Historically, the referer argument wasn't even introduced for
    addon menus, but for the new document and wizards menus, which
    used to be managed by the menu manager back then. This can be
    seen in commit 40fefd8e0d5937666129278fe2b27c36cb58033c ("support
    for images and target frames"). Only later this code path was
    reused for addon menus. That's why the member was called
    m_bIsBookmarkMenu, as "bookmark menu" was the term used for the
    new and wizard menus, and there was also a related BmkMenu class.)
    
    Change-Id: Idd48a0416f8703ef1a5c91e949345537ec9a5ec0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102136
    Tested-by: Jenkins
    Reviewed-by: Maxim Monastirsky <momonasmon at gmail.com>

diff --git a/framework/inc/menuconfiguration.hxx b/framework/inc/menuconfiguration.hxx
index 84943f5326a6..ea3acc41a2f0 100644
--- a/framework/inc/menuconfiguration.hxx
+++ b/framework/inc/menuconfiguration.hxx
@@ -29,8 +29,6 @@ namespace com::sun::star::io { class XInputStream; }
 namespace com::sun::star::io { class XOutputStream; }
 namespace com::sun::star::uno { class XComponentContext; }
 
-const sal_uInt16 ITEMID_ADDONLIST           = 6678; // used to be a SID in sfx2, now just a unique id...
-
 namespace framework
 {
 
diff --git a/framework/inc/uielement/menubarmanager.hxx b/framework/inc/uielement/menubarmanager.hxx
index eb2cb24a85b2..c688994313b8 100644
--- a/framework/inc/uielement/menubarmanager.hxx
+++ b/framework/inc/uielement/menubarmanager.hxx
@@ -65,13 +65,6 @@ class MenuBarManager final :
         css::ui::XUIConfigurationListener,
         css::awt::XSystemDependentMenuPeer>
 {
-        MenuBarManager(
-            const css::uno::Reference< css::uno::XComponentContext >& xContext,
-            const css::uno::Reference< css::frame::XFrame >& rFrame,
-            const css::uno::Reference< css::util::XURLTransformer >& _xURLTransformer,
-            Menu*           pAddonMenu,
-            bool            popup);
-
     public:
         MenuBarManager(
             const css::uno::Reference< css::uno::XComponentContext >& xContext,
@@ -172,12 +165,10 @@ class MenuBarManager final :
         bool         CreatePopupMenuController( MenuItemHandler* pMenuItemHandler );
         void             AddMenu(MenuBarManager* pSubMenuManager,const OUString& _sItemCommand,sal_uInt16 _nItemId);
         sal_uInt16           FillItemCommand(OUString& _rItemCommand, Menu* _pMenu,sal_uInt16 _nIndex) const;
-        void             Init(const css::uno::Reference< css::frame::XFrame >& rFrame,Menu* pAddonMenu,bool _bHandlePopUp);
         void             SetHdl();
 
         bool                                                         m_bDeleteMenu;
         bool                                                         m_bActive;
-        bool                                                         m_bIsBookmarkMenu;
         bool                                                         m_bShowMenuImages;
         bool                                                         m_bRetrieveImages;
         bool                                                         m_bAcceleratorCfg;
diff --git a/framework/source/uielement/menubarmanager.cxx b/framework/source/uielement/menubarmanager.cxx
index 4e2b602306cd..f72e5471d36a 100644
--- a/framework/source/uielement/menubarmanager.cxx
+++ b/framework/source/uielement/menubarmanager.cxx
@@ -72,6 +72,7 @@ using namespace ::com::sun::star::lang;
 using namespace ::com::sun::star::ui;
 
 const sal_uInt16 ADDONMENU_MERGE_ITEMID_START = 1500;
+const sal_uInt16 ITEMID_ADDONLIST             = 6678; // used to be a SID in sfx2, now just a unique id...
 
 namespace framework
 {
@@ -102,25 +103,6 @@ MenuBarManager::MenuBarManager(
     FillMenuManager( pMenu, rFrame, rDispatchProvider, rModuleIdentifier, bDelete );
 }
 
-MenuBarManager::MenuBarManager(
-    const Reference< XComponentContext >& rxContext,
-    const Reference< XFrame >& rFrame,
-    const Reference< XURLTransformer >& _xURLTransformer,
-    Menu* pAddonMenu,
-    bool popup):
-    WeakComponentImplHelper( m_aMutex )
-    , m_bRetrieveImages( true )
-    , m_bAcceleratorCfg( false )
-    , m_bModuleIdentified( false )
-    , m_bHasMenuBar( true )
-    , m_xContext(rxContext)
-    , m_xURLTransformer(_xURLTransformer)
-    , m_sIconTheme( SvtMiscOptions().GetIconTheme() )
-{
-    m_aAsyncSettingsTimer.SetDebugName( "framework::MenuBarManager::Deactivate m_aAsyncSettingsTimer" );
-    Init(rFrame,pAddonMenu, popup);
-}
-
 Any SAL_CALL MenuBarManager::getMenuHandle( const Sequence< sal_Int8 >& /*ProcessId*/, sal_Int16 SystemType )
 {
     SolarMutexGuard aSolarGuard;
@@ -695,10 +677,8 @@ IMPL_LINK( MenuBarManager, Activate, Menu *, pMenu, bool )
 
                         if ( aTargetURL.Complete.startsWith( ".uno:StyleApply?" ) )
                             xMenuItemDispatch = new StyleDispatcher( m_xFrame, m_xURLTransformer, aTargetURL );
-                        else if ( m_bIsBookmarkMenu )
-                            xMenuItemDispatch = xDispatchProvider->queryDispatch( aTargetURL, menuItemHandler->aTargetFrame, 0 );
                         else
-                            xMenuItemDispatch = xDispatchProvider->queryDispatch( aTargetURL, OUString(), 0 );
+                            xMenuItemDispatch = xDispatchProvider->queryDispatch( aTargetURL, menuItemHandler->aTargetFrame, 0 );
 
                         bool bPopupMenu( false );
                         if ( !menuItemHandler->xPopupMenuController.is() &&
@@ -819,9 +799,9 @@ IMPL_LINK( MenuBarManager, Select, Menu *, pMenu, bool )
                 aTargetURL.Complete = pMenuItemHandler->aMenuItemURL;
                 m_xURLTransformer->parseStrict( aTargetURL );
 
-                if ( m_bIsBookmarkMenu )
+                if ( pMenu->GetUserValue( nCurItemId ) )
                 {
-                    // bookmark menu item selected
+                    // addon menu item selected
                     aArgs.realloc( 1 );
                     aArgs[0].Name = "Referer";
                     aArgs[0].Value <<= OUString( "private:user" );
@@ -941,7 +921,6 @@ void MenuBarManager::FillMenuManager( Menu* pMenu, const Reference< XFrame >& rF
     m_bActive           = false;
     m_bDeleteMenu       = bDelete;
     m_pVCLMenu          = pMenu;
-    m_bIsBookmarkMenu   = false;
     m_xDispatchProvider = rDispatchProvider;
 
     const StyleSettings& rSettings = Application::GetSettings().GetStyleSettings();
@@ -1036,12 +1015,6 @@ void MenuBarManager::FillMenuManager( Menu* pMenu, const Reference< XFrame >& rF
                 }
                 lcl_CheckForChildren(pMenu, nItemId);
             }
-            else if ( aItemCommand.startsWith( ADDONSPOPUPMENU_URL_PREFIX_STR ) )
-            {
-                // A special addon popup menu, must be created with a different ctor
-                MenuBarManager* pSubMenuManager = new MenuBarManager( m_xContext, m_xFrame, m_xURLTransformer, pPopup, true );
-                AddMenu(pSubMenuManager,aItemCommand,nItemId);
-            }
             else
             {
                 Reference< XDispatchProvider > xPopupMenuDispatchProvider( rDispatchProvider );
@@ -1051,11 +1024,8 @@ void MenuBarManager::FillMenuManager( Menu* pMenu, const Reference< XFrame >& rF
                 if ( pAttributes )
                     xPopupMenuDispatchProvider = pAttributes->xDispatchProvider;
 
-                // Check if this is the help menu. Add menu item if needed
-                if ( aItemCommand == aCmdHelpMenu )
-                {
-                }
-                else if ( aItemCommand == aCmdToolsMenu && AddonMenuManager::HasAddonMenuElements() )
+                // Check if this is the tools menu. Add menu item if needed
+                if ( aItemCommand == aCmdToolsMenu && AddonMenuManager::HasAddonMenuElements() )
                 {
                     // Create addon popup menu if there exist elements and this is the tools popup menu
                     VclPtr<PopupMenu> pSubMenu = AddonMenuManager::CreateAddonMenu(rFrame);
@@ -1072,13 +1042,8 @@ void MenuBarManager::FillMenuManager( Menu* pMenu, const Reference< XFrame >& rF
                         pSubMenu.disposeAndClear();
                 }
 
-                MenuBarManager* pSubMenuManager;
-                if ( nItemId == ITEMID_ADDONLIST )
-                    pSubMenuManager = new MenuBarManager( m_xContext, m_xFrame, m_xURLTransformer, pPopup, false );
-                else
-                    pSubMenuManager = new MenuBarManager( m_xContext, rFrame, m_xURLTransformer,
-                                                          rDispatchProvider, aModuleIdentifier,
-                                                          pPopup, false, m_bHasMenuBar );
+                MenuBarManager* pSubMenuManager = new MenuBarManager( m_xContext, rFrame, m_xURLTransformer,
+                    rDispatchProvider, aModuleIdentifier, pPopup, false, m_bHasMenuBar );
 
                 AddMenu(pSubMenuManager, aItemCommand, nItemId);
             }
@@ -1089,6 +1054,10 @@ void MenuBarManager::FillMenuManager( Menu* pMenu, const Reference< XFrame >& rF
                 m_bRetrieveImages = true;
 
             std::unique_ptr<MenuItemHandler> pItemHandler(new MenuItemHandler( nItemId, xStatusListener, xDispatch ));
+            // Retrieve possible attributes struct
+            MenuAttributes* pAttributes = static_cast<MenuAttributes *>(pMenu->GetUserValue( nItemId ));
+            if ( pAttributes )
+                pItemHandler->aTargetFrame = pAttributes->aTargetFrame;
             pItemHandler->aMenuItemURL = aItemCommand;
 
             if ( m_xPopupMenuControllerFactory.is() &&
@@ -1623,81 +1592,6 @@ sal_uInt16 MenuBarManager::FillItemCommand(OUString& _rItemCommand, Menu* _pMenu
     }
     return nItemId;
 }
-void MenuBarManager::Init(const Reference< XFrame >& rFrame, Menu* pAddonMenu, bool _bHandlePopUp)
-{
-    m_bActive           = false;
-    m_bDeleteMenu       = false;
-    m_pVCLMenu          = pAddonMenu;
-    m_xFrame            = rFrame;
-    m_bIsBookmarkMenu   = true;
-    m_bShowMenuImages   = true;
-
-    m_xPopupMenuControllerFactory = frame::thePopupMenuControllerFactory::get(
-        ::comphelper::getProcessComponentContext());
-
-    Reference< XStatusListener > xStatusListener;
-    Reference< XDispatch > xDispatch;
-    sal_uInt16 nItemCount = pAddonMenu->GetItemCount();
-    OUString aItemCommand;
-    m_aMenuItemHandlerVector.reserve(nItemCount);
-    for ( sal_uInt16 i = 0; i < nItemCount; i++ )
-    {
-        sal_uInt16 nItemId = FillItemCommand(aItemCommand,pAddonMenu, i );
-
-        PopupMenu* pPopupMenu = pAddonMenu->GetPopupMenu( nItemId );
-        if ( pPopupMenu )
-        {
-            Reference< XDispatchProvider > xDispatchProvider;
-            MenuBarManager* pSubMenuManager = new MenuBarManager( m_xContext, rFrame, m_xURLTransformer,
-                                                                  xDispatchProvider, OUString(), pPopupMenu,
-                                                                  false );
-
-            Reference< XStatusListener > xSubMenuManager( static_cast< OWeakObject *>( pSubMenuManager ), UNO_QUERY );
-
-            std::unique_ptr<MenuItemHandler> pMenuItemHandler(new MenuItemHandler(
-                                                        nItemId,
-                                                        xSubMenuManager,
-                                                        xDispatch ));
-            pMenuItemHandler->aMenuItemURL = aItemCommand;
-            m_aMenuItemHandlerVector.push_back( std::move(pMenuItemHandler) );
-        }
-        else
-        {
-            if ( pAddonMenu->GetItemType( i ) != MenuItemType::SEPARATOR )
-            {
-                MenuAttributes* pAddonAttributes = static_cast<MenuAttributes *>(pAddonMenu->GetUserValue( nItemId ));
-                std::unique_ptr<MenuItemHandler> pMenuItemHandler(new MenuItemHandler( nItemId, xStatusListener, xDispatch ));
-
-                if ( pAddonAttributes )
-                {
-                    // read additional attributes from attributes struct and AddonMenu implementation
-                    // will delete all attributes itself!!
-                    pMenuItemHandler->aTargetFrame = pAddonAttributes->aTargetFrame;
-                }
-
-                pMenuItemHandler->aMenuItemURL = aItemCommand;
-                if ( _bHandlePopUp )
-                {
-                    // Check if we have to create a popup menu for a uno based popup menu controller.
-                    // We have to set an empty popup menu into our menu structure so the controller also
-                    // works with inplace OLE.
-                    if ( m_xPopupMenuControllerFactory.is() &&
-                         m_xPopupMenuControllerFactory->hasController( aItemCommand, m_aModuleIdentifier ) )
-                    {
-                        VCLXPopupMenu* pVCLXPopupMenu = new VCLXPopupMenu;
-                        PopupMenu* pCtlPopupMenu = static_cast<PopupMenu *>(pVCLXPopupMenu->GetMenu());
-                        pAddonMenu->SetPopupMenu( pMenuItemHandler->nItemId, pCtlPopupMenu );
-                        pMenuItemHandler->xPopupMenu = pVCLXPopupMenu;
-
-                    }
-                }
-                m_aMenuItemHandlerVector.push_back( std::move(pMenuItemHandler) );
-            }
-        }
-    }
-
-    SetHdl();
-}
 
 void MenuBarManager::SetHdl()
 {


More information about the Libreoffice-commits mailing list