framework/source/accelerators/acceleratorcache.cxx | 48 ++++++++++-------
framework/source/inc/accelerators/acceleratorcache.hxx | 4 +
2 files changed, 35 insertions(+), 17 deletions(-)
New commits:
commit 4be9be221922db6e1872e061b312f3942bca900e
Author: Neil Roberts <bpee...@yahoo.co.uk>
AuthorDate: Sat Sep 13 19:30:07 2025 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Sun Sep 14 21:14:06 2025 +0200
tdf#144560 Remove old key from command when setting new one
The accelerator cache maintains a 1:1 mapping for key→command and a
1:n mapping for command→keys. When a new command is set for a key it was
previously only updating the key→command map and then adding the key to
the command→keys map. It never removed the key from the command→keys map
for the old command. When the menus are eventually updated, it looks up
whether each item has a shortcut based on the item’s command. Because
the old command was never removed, this ended up making it display the
same key binding on multiple menu items.
Change-Id: I6d75834f4c49b1f5c105ec40cb27b8e73006152d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/190928
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/framework/source/accelerators/acceleratorcache.cxx
b/framework/source/accelerators/acceleratorcache.cxx
index 8f64ed330e19..4b993d9f232a 100644
--- a/framework/source/accelerators/acceleratorcache.cxx
+++ b/framework/source/accelerators/acceleratorcache.cxx
@@ -48,8 +48,38 @@ AcceleratorCache::TKeyList AcceleratorCache::getAllKeys()
const
return lKeys;
}
+void AcceleratorCache::removeKeyForCommand(const OUString& sCommand, const
css::awt::KeyEvent& aKey)
+{
+ // get keylist for that command
+ TCommand2Keys::iterator pCommand = m_lCommand2Keys.find(sCommand);
+ if (pCommand == m_lCommand2Keys.end())
+ return;
+ TKeyList& lKeys = pCommand->second;
+
+ // one or more keys assign
+ if (lKeys.size() == 1)
+ // remove key from optimized command list
+ m_lCommand2Keys.erase(sCommand);
+ else // only remove this key from the keylist
+ {
+ auto pKeys = ::std::find(lKeys.begin(), lKeys.end(), aKey);
+
+ if (pKeys != lKeys.end())
+ lKeys.erase(pKeys);
+ }
+}
+
void AcceleratorCache::setKeyCommandPair(const css::awt::KeyEvent& aKey, const
OUString& sCommand)
{
+ // remove any previous binding for the key
+ if (auto oldCommandPos = m_lKey2Commands.find(aKey); oldCommandPos !=
m_lKey2Commands.end())
+ {
+ // Take another ref to the command because the other string is
+ // the key in the map and it’s going to get destroyed.
+ OUString oldCommand = oldCommandPos->second;
+ removeKeyForCommand(oldCommand, aKey);
+ }
+
// register command for the specified key
m_lKey2Commands[aKey] = sCommand;
@@ -90,23 +120,7 @@ void AcceleratorCache::removeKey(const css::awt::KeyEvent&
aKey)
// remove key from primary list
m_lKey2Commands.erase(aKey);
- // get keylist for that command
- TCommand2Keys::iterator pCommand = m_lCommand2Keys.find(sCommand);
- if (pCommand == m_lCommand2Keys.end())
- return;
- TKeyList& lKeys = pCommand->second;
-
- // one or more keys assign
- if (lKeys.size() == 1)
- // remove key from optimized command list
- m_lCommand2Keys.erase(sCommand);
- else // only remove this key from the keylist
- {
- auto pKeys = ::std::find(lKeys.begin(), lKeys.end(), aKey);
-
- if (pKeys != lKeys.end())
- lKeys.erase(pKeys);
- }
+ removeKeyForCommand(sCommand, aKey);
}
void AcceleratorCache::removeCommand(const OUString& sCommand)
diff --git a/framework/source/inc/accelerators/acceleratorcache.hxx
b/framework/source/inc/accelerators/acceleratorcache.hxx
index adfdf8fe5aa2..a8ada0d5a9a4 100644
--- a/framework/source/inc/accelerators/acceleratorcache.hxx
+++ b/framework/source/inc/accelerators/acceleratorcache.hxx
@@ -108,6 +108,10 @@ class AcceleratorCache
OUString getCommandByKey(const css::awt::KeyEvent& aKey) const;
void removeKey(const css::awt::KeyEvent& aKey);
void removeCommand(const OUString& sCommand);
+
+ private:
+ void removeKeyForCommand(const OUString& sCommand,
+ const css::awt::KeyEvent& aKey);
};
} // namespace framework
