From 5b4ce2be04f00e001bad6c4d6c59ff3791b1125c Mon Sep 17 00:00:00 2001 From: dghart Date: Wed, 10 Dec 2014 20:42:12 +0000 Subject: [PATCH] When getting a pointer to a menubar menu, don't search by title Using wxMenuBar::FindMenu to find the index of a menu isn't as mnemonic-safe as it should be. There are two problems: 1) If the search string isn't localised, it will fail if there's a translation. 2) Even if it is, there's a potential problem with mnemonic location, as translators are allowed to use a non-default letter. This _shouldn't_ matter as wx removes the mnemonic before matching. However it seems that in Japanese, the mnemonic is appended (e.g. Bookmarks(&B)) and unsurprisingly wxStripMenuCodes() can't cope with this. So store the position while loading the menubar; which isn't elegant, but it is safe. --- Accelerators.cpp | 4 ++++ Bookmarks.cpp | 13 +++++++++---- Bookmarks.h | 5 ++++- Tools.cpp | 14 ++++++++------ Tools.h | 3 +++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Accelerators.cpp b/Accelerators.cpp index eb75627..d470c91 100644 --- a/Accelerators.cpp +++ b/Accelerators.cpp @@ -16,6 +16,7 @@ #include "Externs.h" #include "Configure.h" +#include "Bookmarks.h" #include "Accelerators.h" @@ -490,6 +491,9 @@ subitemslistarray[0] = columnitems; subitemslistcount[0] = sizeof(columnitems)/p // Make arrays for menus & for submenus const wxString menutitles[] = { _("&File"), _("&Edit"), _("&View"), _("&Tabs"), _("&Bookmarks"), _("&Archive"), _("&Mount"), _("Too&ls"), _("&Options"), _("&Help") }; +Bookmarks::SetMenuIndex(4); // *** remember to change these if their position changes! *** +LaunchMiscTools::SetMenuIndex(7); + const wxString submenutitles[] = { _("&Columns to Display"), _("&Load a Tab Template") }; int submenID[2]; submenID[0] = SHCUT_FINISH + 1; submenID[1] = LoadTabTemplateMenu; // "Columns to Display" hasn't a set id, so use SHCUT_FINISH+1 wxMenu* menuarray[ menuno ]; wxMenu* submenuarray[ submenuno ]; diff --git a/Bookmarks.cpp b/Bookmarks.cpp index e0e2e7d..65ae21c 100644 --- a/Bookmarks.cpp +++ b/Bookmarks.cpp @@ -318,6 +318,8 @@ END_EVENT_TABLE() //-------------------------------------------------------------------------------------------------------------------- +int Bookmarks::m_menuindex = -1; + Bookmarks::Bookmarks() { LoadBookmarks(); @@ -336,8 +338,9 @@ MenuStruct.ClearData(); // Make sure the arrays are em bookmarkID = ID_FIRST_BOOKMARK; // Similarly reset bookmarkID MenuStruct.ID = GetNextID(); // & use it indirectly to id MenuStruct -wxMenu* menu = MenuBar->GetMenu(MenuBar->FindMenu(_("Bookmarks"))); // Find the menu for the group -if (menu==NULL) { wxLogError(_("Couldn't find menu!?")); return; } +wxCHECK_RET(m_menuindex > wxNOT_FOUND, wxT("Bookmarks menu index not set")); // Should have been set when the bar was loaded +wxMenu* menu = MenuBar->GetMenu(m_menuindex); // Find the menu for the group +if (!menu) { wxLogError(_("Couldn't find menu!?")); return; } MenuStruct.pathname = wxT("/Bookmarks"); MenuStruct.FullLabel.Empty(); // Needed for re-entry during Save/Load @@ -435,8 +438,10 @@ config->Flush(); wxMenuBar* MenuBar = MyFrame::mainframe->GetMenuBar(); // We now have to destroy the menu structure, so it can be born again if (MenuBar==NULL) { wxLogError(_("Couldn't load Menubar!?")); return; } - -wxMenu* mainmenu = MenuBar->GetMenu(MenuBar->FindMenu(_("Bookmarks"))); // Find the menu for the group + +wxCHECK_RET(m_menuindex > wxNOT_FOUND, wxT("Bookmarks menu index not set")); // Should have been set when the bar was loaded +wxMenu* mainmenu = MenuBar->GetMenu(m_menuindex); // Find the menu for the group +if (!mainmenu) { wxLogError(_("Couldn't find menu!?")); return; } for (size_t count=mainmenu->GetMenuItemCount(); count > 3 ; --count) // Skip the 1st 3 items, they're Add, Manage & a Separator mainmenu->Destroy(mainmenu->GetMenuItems().Item(count-1)->GetData()); // Destroy all the others, including submenus diff --git a/Bookmarks.h b/Bookmarks.h index b476a4d..a52109c 100644 --- a/Bookmarks.h +++ b/Bookmarks.h @@ -173,6 +173,8 @@ bool Cut(wxTreeItemId item); bool Paste(wxTreeItemId item, bool NoDupCheck=false, bool duplicating=false); bool Copy(wxTreeItemId item); +static void SetMenuIndex(int index) { m_menuindex = index; } + wxDialog* adddlg; // These 2 ptrs are used to access their dialogs from other methods MyBookmarkDialog* mydlg; BMClipboard bmclip; // Stores the TreeItemData & structs for pasting @@ -182,7 +184,8 @@ wxConfigBase* config; MyBmTree* tree; // The treectrl used in ManageBookmarks bool m_altered; // Do we have anything to save? -unsigned int bookmarkID; // Actually it ID's folders & separators too +static int m_menuindex; // Passed to wxMenuBar::GetMenu so we can ID the menu without using its label +unsigned int bookmarkID; // Actually it ID.s folders & separators too unsigned int GetNextID(){ if (bookmarkID == ID__LAST_BOOKMARK) bookmarkID=ID_FIRST_BOOKMARK; // Recycle return bookmarkID++; } // Return next vacant bookmarkID diff --git a/Tools.cpp b/Tools.cpp index 60b5879..f5ec1fe 100644 --- a/Tools.cpp +++ b/Tools.cpp @@ -2678,6 +2678,8 @@ BriefLogStatus bls(msg + msgA + msgB); } //----------------------------------------------------------------------------------------------------------------------- +int LaunchMiscTools::m_menuindex = -1; + void LaunchMiscTools::ClearData() { for (int n = (int)ToolArray.GetCount(); n > 0; --n) { UserDefinedTool* item = ToolArray.Item(n-1); delete item; ToolArray.RemoveAt(n-1); } @@ -2691,9 +2693,9 @@ LoadArrays(); if (!FolderArray.GetCount()) return; // No data, not event the "Run a Program" menu if (tools == NULL) // If so, we're loading the menubar menu, so find it - { int id = MyFrame::mainframe->GetMenuBar()->FindMenu(_("Too&ls")); // Find the Tools menu id - if (id == wxNOT_FOUND) return; - tools = MyFrame::mainframe->GetMenuBar()->GetMenu(id); // Get ptr to the menu itself + { wxCHECK_RET(m_menuindex > wxNOT_FOUND, wxT("Tools menu index not set")); // Should have been set when the bar was loaded + tools = MyFrame::mainframe->GetMenuBar()->GetMenu(m_menuindex); + wxCHECK_RET(tools, wxT("Couldn't find the Tools menu")); static bool alreadyloaded=false; if (!alreadyloaded) { tools->AppendSeparator(); alreadyloaded=true; } // We don't want multiple separators due to reloads } @@ -2777,9 +2779,9 @@ config->DeleteGroup(wxT("/Tools/Launch")); // Delete current info (oth Save(wxT("/Tools/Launch"), config); // Recursively save config->Flush(); -int id = MyFrame::mainframe->GetMenuBar()->FindMenu(_("Tools")); // Find the id of the Tools menu proper -if (id == wxNOT_FOUND) return; -wxMenu* tools = MyFrame::mainframe->GetMenuBar()->GetMenu(id); // & hence the menu itself +wxCHECK_RET(m_menuindex > wxNOT_FOUND, wxT("Tools menu index not set")); // Should have been set when the bar was loaded +wxMenu* tools = MyFrame::mainframe->GetMenuBar()->GetMenu(m_menuindex); +wxCHECK_RET(tools, wxT("Couldn't find the Tools menu")); int sm = tools->FindItem(FolderArray[0]->Label); // Find the id menu of the relevant submenu in the Tools menu if (sm == wxNOT_FOUND) return; diff --git a/Tools.h b/Tools.h index d6a7f58..6554a82 100644 --- a/Tools.h +++ b/Tools.h @@ -494,6 +494,8 @@ ArrayOfUserDefinedTools* GetToolarray(){ return &ToolArray; } ArrayOfToolFolders* GetFolderArray(){ return &FolderArray; } size_t GetLastCommand(){ return LastCommandNo; } +static void SetMenuIndex(int index) { m_menuindex = index; } + protected: void Load(wxString name); // Load (sub)menu 'name' from config void ClearData(); @@ -505,6 +507,7 @@ bool SetCorrectPaneptrs(const wxChar* &pc, MyGenericDirCtrl* &first, MyGenericDi ArrayOfUserDefinedTools ToolArray; ArrayOfToolFolders FolderArray; int EventId; // Holds the first spare ID, from SHCUT_TOOLS_LAUNCH to SHCUT_TOOLS_LAUNCH_LAST-1 +static int m_menuindex; // Passed to wxMenuBar::GetMenu so we can ID the menu without using its label size_t EntriesIndex; size_t FolderIndex; size_t LastCommandNo; // Holds the last tool used, so it can be repeated -- 2.1.0