diff options
| author | Maxim Monastirsky <momonasmon@gmail.com> | 2016-01-22 01:51:46 +0200 |
|---|---|---|
| committer | Andras Timar <andras.timar@collabora.com> | 2016-02-02 14:20:17 +0100 |
| commit | a71662839b67b4a08f02ddf9ee00385a21d10881 (patch) | |
| tree | 40412e07356bbe513d9bf7e222838ca300b5f7a3 /framework | |
| parent | dcb18e5de29adc489ea8743b0247458be677162f (diff) | |
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.
(cherry picked from commit 0dbe3d40579d20f4cbce3ce155996ff4b5c32c99)
Conflicts:
framework/source/fwe/xml/menudocumenthandler.cxx
Change-Id: I2f88807eceae1006066a14750f2003e235f49ad4
Reviewed-on: https://gerrit.libreoffice.org/21705
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit 23d363f019e8c6911adb5f8b07bdb7ff09d467cc)
Diffstat (limited to 'framework')
| -rw-r--r-- | framework/source/fwe/xml/menudocumenthandler.cxx | 8 |
1 files changed, 3 insertions, 5 deletions
diff --git a/framework/source/fwe/xml/menudocumenthandler.cxx b/framework/source/fwe/xml/menudocumenthandler.cxx index 9f48ff4ad901..ccb5b5a39fa6 100644 --- a/framework/source/fwe/xml/menudocumenthandler.cxx +++ b/framework/source/fwe/xml/menudocumenthandler.cxx @@ -76,8 +76,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"; @@ -814,7 +812,7 @@ throw ( SAXException, RuntimeException ) m_aAttributeType, aCommandURL ); - if ( aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL ) + if ( !aLabel.isEmpty() ) pListMenu->AddAttribute( OUString( ATTRIBUTE_NS_LABEL ), m_aAttributeType, aLabel ); @@ -872,13 +870,13 @@ void OWriteMenuDocumentHandler::WriteMenuItem( const OUString& aCommandURL, cons aHelpURL ); } - if ( !aLabel.isEmpty() && aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL ) + if ( !aLabel.isEmpty() ) { pList->AddAttribute( OUString( ATTRIBUTE_NS_LABEL ), m_aAttributeType, aLabel ); } - if (( nStyle > 0 ) && aCommandURL.copy( CMD_PROTOCOL_SIZE ) != CMD_PROTOCOL ) + if ( nStyle > 0 ) { OUString aValue; MenuStyleItem* pStyle = MenuItemStyles; |