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

[Maxim Monastirsky]

 framework/source/fwe/xml/menudocumenthandler.cxx |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

New commits:
commit 2a74b694dbbca2083ffa4b20ec89c8574f00c34a
Author: Maxim Monastirsky <momonasmon at gmail.com>
Date:   Fri Jan 22 01:51:46 2016 +0200

    Fix wrong use of OUString::copy
    
    Code like:
    
    if( aCommandURL.copy(5) != ".uno:" )
    
    is obviously wrong, as OUString::copy(sal_Int32) takes the _beginning_
    index, so for this condition to be false the command URL must have
    ".uno:" in the _middle_ of the string. This created some weird things
    like an empty label attribute added to any submenu item. Moreover, the
    command URL can be easily shorter than 5 (like when a custom submenu
    added by the user). Using copy(5) in such case officially considered as
    "undefined behavior" and will trigger an assert in debug build (that's
    how I discovered this code actually).
    
    Most likely the original intent was to check whether the command URL
    doesn't start with ".uno:", and so should be changed to use
    OUString::startsWith. But doing that will create a regression, as it
    won't be possible anymore to change labels of commands that start with
    ".uno:". Simply dropping this check seems to be better solution here.
    
    Change-Id: I2f88807eceae1006066a14750f2003e235f49ad4
    (cherry picked from commit 0dbe3d40579d20f4cbce3ce155996ff4b5c32c99)
    Reviewed-on: https://gerrit.libreoffice.org/21704
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/framework/source/fwe/xml/menudocumenthandler.cxx b/framework/source/fwe/xml/menudocumenthandler.cxx
index 1d7122e..5f59258 100644
--- a/framework/source/fwe/xml/menudocumenthandler.cxx
+++ b/framework/source/fwe/xml/menudocumenthandler.cxx
@@ -80,8 +80,6 @@ static const char ITEM_DESCRIPTOR_TYPE[]        = "Type";
 static const char ITEM_DESCRIPTOR_STYLE[]       = "Style";
 
 // special popup menus (filled during runtime) must be saved as an empty popup menu or menuitem!!!
-static const sal_Int32 CMD_PROTOCOL_SIZE        = 5;
-static const char CMD_PROTOCOL[]                = ".uno:";
 static const char ADDDIRECT_CMD[]               = ".uno:AddDirect";
 static const char AUTOPILOTMENU_CMD[]           = ".uno:AutoPilotMenu";
 
@@ -839,7 +837,7 @@ throw ( SAXException, RuntimeException )
                                             m_aAttributeType,
                                             aCommandURL );
 
-                    if ( aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL )
+                    if ( !aLabel.isEmpty() )
                         pListMenu->AddAttribute( ATTRIBUTE_NS_LABEL,
                                                  m_aAttributeType,
                                                  aLabel );
@@ -897,13 +895,13 @@ void OWriteMenuDocumentHandler::WriteMenuItem( const OUString& aCommandURL, cons
                              aHelpURL );
     }
 
-    if ( !aLabel.isEmpty() && aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL )
+    if ( !aLabel.isEmpty() )
     {
         pList->AddAttribute( ATTRIBUTE_NS_LABEL,
                                 m_aAttributeType,
                                 aLabel );
     }
-    if (( nStyle > 0 ) && aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL )
+    if ( nStyle > 0 )
     {
         OUString aValue;
         MenuStyleItem* pStyle = MenuItemStyles;