From 9907ca13f9baa04f5d4cc3d5b6bc13d1fdbf147e Mon Sep 17 00:00:00 2001 From: utelle Date: Fri, 31 Aug 2018 16:23:05 +0200 Subject: [PATCH 1/3] Fix issue with MDI window menu label If the locale is changed while the MDI window menu is active, the menubar entry labelled with the previous translation will not be removed. The previous translation is now remembered and used to locate the menubar entry. --- src/msw/mdi.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index b520a01bb0..a5cc0e98a4 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -77,6 +77,7 @@ const int wxID_MDI_MORE_WINDOWS = wxLAST_MDI_CHILD + 1; // The MDI "Window" menu label const char *WINDOW_MENU_LABEL = gettext_noop("&Window"); +wxString WINDOW_MENU_LABEL_TRANSLATED; // --------------------------------------------------------------------------- // private functions @@ -1562,6 +1563,7 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) bool inserted = false; wxString buf; MenuIterator it(hmenu); + WINDOW_MENU_LABEL_TRANSLATED = wxGetTranslation(WINDOW_MENU_LABEL); while ( it.GetNext(buf) ) { const wxString label = wxStripMenuCodes(buf); @@ -1571,7 +1573,7 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) ::InsertMenu(hmenu, it.GetPos(), MF_BYPOSITION | MF_POPUP | MF_STRING, (UINT_PTR)menuWin, - wxString(wxGetTranslation(WINDOW_MENU_LABEL)).t_str()); + WINDOW_MENU_LABEL_TRANSLATED.t_str()); break; } } @@ -1580,7 +1582,7 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) { ::AppendMenu(hmenu, MF_POPUP, (UINT_PTR)menuWin, - wxString(wxGetTranslation(WINDOW_MENU_LABEL)).t_str()); + WINDOW_MENU_LABEL_TRANSLATED.t_str()); } } @@ -1597,7 +1599,7 @@ void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu) MenuIterator it(hmenu); while ( it.GetNext(buf) ) { - if ( wxStrcmp(buf, wxGetTranslation(WINDOW_MENU_LABEL)) == 0 ) + if ( wxStrcmp(buf, WINDOW_MENU_LABEL_TRANSLATED) == 0 ) { if ( !::RemoveMenu(hmenu, it.GetPos(), MF_BYPOSITION) ) { From 9a9176a1be12793986b69a712adf0b8fca721d0c Mon Sep 17 00:00:00 2001 From: utelle Date: Fri, 31 Aug 2018 23:28:53 +0200 Subject: [PATCH 2/3] Change storage of window menu label translation Use member variable instead of global variable --- include/wx/msw/mdi.h | 9 +++++++++ src/msw/mdi.cpp | 39 +++++++++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/include/wx/msw/mdi.h b/include/wx/msw/mdi.h index 0d77639fdd..9c1d1c6372 100644 --- a/include/wx/msw/mdi.h +++ b/include/wx/msw/mdi.h @@ -85,6 +85,12 @@ public: virtual void RemoveMDIChild(wxMDIChildFrame *child); #endif // wxUSE_MENUS + // called by AddWindowMenu to remember the translation of the window menu label + void SetWindowMenuLabelTranslated(); + + // called by AddWindowMenu and RemoveWindowMenu to get the last used translation of the window menu label + const wxString GetWindowMenuLabelTranslated() const; + // handlers // -------- @@ -150,6 +156,9 @@ private: // it was "handled", see OnActivate() and HandleActivate() bool m_activationNotHandled; + // holds the current translation for the window menu label + wxString m_windowMenuLabelTranslated; + friend class WXDLLIMPEXP_FWD_CORE wxMDIChildFrame; diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index a5cc0e98a4..7ddd464dfe 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -77,7 +77,6 @@ const int wxID_MDI_MORE_WINDOWS = wxLAST_MDI_CHILD + 1; // The MDI "Window" menu label const char *WINDOW_MENU_LABEL = gettext_noop("&Window"); -wxString WINDOW_MENU_LABEL_TRANSLATED; // --------------------------------------------------------------------------- // private functions @@ -89,10 +88,10 @@ void MDISetMenu(wxWindow *win, HMENU hmenuFrame, HMENU hmenuWindow); // insert the window menu (subMenu) into menu just before "Help" submenu or at // the very end if not found -void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU subMenu); +void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU subMenu, const wxString& windowMenuLabelTranslated); // Remove the window menu -void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu); +void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu, const wxString& windowMenuLabelTranslated); // unpack the parameters of WM_MDIACTIVATE message void UnpackMDIActivate(WXWPARAM wParam, WXLPARAM lParam, @@ -278,6 +277,16 @@ int wxMDIParentFrame::GetChildFramesCount() const return count; } +void wxMDIParentFrame::SetWindowMenuLabelTranslated() +{ + m_windowMenuLabelTranslated = wxGetTranslation(WINDOW_MENU_LABEL); +} + +const wxString wxMDIParentFrame::GetWindowMenuLabelTranslated() const +{ + return m_windowMenuLabelTranslated; +} + #if wxUSE_MENUS void wxMDIParentFrame::AddMDIChild(wxMDIChildFrame * WXUNUSED(child)) @@ -335,7 +344,8 @@ void wxMDIParentFrame::AddWindowMenu() // attach it to the menu bar. m_windowMenu->Attach(GetMenuBar()); - MDIInsertWindowMenu(GetClientWindow(), m_hMenu, GetMDIWindowMenu(this)); + SetWindowMenuLabelTranslated(); + MDIInsertWindowMenu(GetClientWindow(), m_hMenu, GetMDIWindowMenu(this), GetWindowMenuLabelTranslated()); } } @@ -343,7 +353,7 @@ void wxMDIParentFrame::RemoveWindowMenu() { if ( m_windowMenu ) { - MDIRemoveWindowMenu(GetClientWindow(), m_hMenu); + MDIRemoveWindowMenu(GetClientWindow(), m_hMenu, GetWindowMenuLabelTranslated()); m_windowMenu->Detach(); } @@ -927,7 +937,7 @@ wxMDIChildFrame::~wxMDIChildFrame() DestroyChildren(); - MDIRemoveWindowMenu(NULL, m_hMenu); + MDIRemoveWindowMenu(NULL, m_hMenu, parent->GetWindowMenuLabelTranslated()); // MDIRemoveWindowMenu() doesn't update the MDI menu when called with NULL // window, so do it ourselves. @@ -1048,12 +1058,14 @@ void wxMDIChildFrame::InternalSetMenuBar() wxMDIParentFrame * const parent = GetMDIParent(); MDIInsertWindowMenu(parent->GetClientWindow(), - m_hMenu, GetMDIWindowMenu(parent)); + m_hMenu, GetMDIWindowMenu(parent), parent->GetWindowMenuLabelTranslated()); } void wxMDIChildFrame::DetachMenuBar() { - MDIRemoveWindowMenu(NULL, m_hMenu); + wxMDIParentFrame * const parent = GetMDIParent(); + + MDIRemoveWindowMenu(NULL, m_hMenu, parent->GetWindowMenuLabelTranslated()); wxFrame::DetachMenuBar(); } @@ -1553,7 +1565,7 @@ private: wxDECLARE_NO_COPY_CLASS(MenuIterator); }; -void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) +void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin, const wxString& windowMenuLabelTranslated) { HMENU hmenu = (HMENU)hMenu; @@ -1563,7 +1575,6 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) bool inserted = false; wxString buf; MenuIterator it(hmenu); - WINDOW_MENU_LABEL_TRANSLATED = wxGetTranslation(WINDOW_MENU_LABEL); while ( it.GetNext(buf) ) { const wxString label = wxStripMenuCodes(buf); @@ -1573,7 +1584,7 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) ::InsertMenu(hmenu, it.GetPos(), MF_BYPOSITION | MF_POPUP | MF_STRING, (UINT_PTR)menuWin, - WINDOW_MENU_LABEL_TRANSLATED.t_str()); + windowMenuLabelTranslated.t_str()); break; } } @@ -1582,14 +1593,14 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) { ::AppendMenu(hmenu, MF_POPUP, (UINT_PTR)menuWin, - WINDOW_MENU_LABEL_TRANSLATED.t_str()); + windowMenuLabelTranslated.t_str()); } } MDISetMenu(win, hmenu, menuWin); } -void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu) +void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu, const wxString& windowMenuLabelTranslated) { HMENU hmenu = (HMENU)hMenu; @@ -1599,7 +1610,7 @@ void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu) MenuIterator it(hmenu); while ( it.GetNext(buf) ) { - if ( wxStrcmp(buf, WINDOW_MENU_LABEL_TRANSLATED) == 0 ) + if ( wxStrcmp(buf, windowMenuLabelTranslated) == 0 ) { if ( !::RemoveMenu(hmenu, it.GetPos(), MF_BYPOSITION) ) { From 2ee322b667d8470ed7439bfef2092f5a5a60291d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 18 Sep 2018 00:13:35 +0200 Subject: [PATCH 3/3] Slightly simplify the previous commits Remove SetWindowMenuLabelTranslated() which was only called once and so didn't seem to be worth having. Also get rid of WINDOW_MENU_LABEL for the same reason: it was used only once and imposed the use of gettext_noop() which is not needed any longer. Rename GetWindowMenuLabelTranslated() to have a MSW prefix in order to indicate that it's a private MSW function and not part of the public wx API. Also renamed "Translated" to "Current" to (hopefully) more clearly indicate why do we need to keep this variable. --- include/wx/msw/mdi.h | 13 +++++++------ src/msw/mdi.cpp | 33 +++++++++++++-------------------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/include/wx/msw/mdi.h b/include/wx/msw/mdi.h index 9c1d1c6372..c0fd25282b 100644 --- a/include/wx/msw/mdi.h +++ b/include/wx/msw/mdi.h @@ -85,11 +85,12 @@ public: virtual void RemoveMDIChild(wxMDIChildFrame *child); #endif // wxUSE_MENUS - // called by AddWindowMenu to remember the translation of the window menu label - void SetWindowMenuLabelTranslated(); - - // called by AddWindowMenu and RemoveWindowMenu to get the last used translation of the window menu label - const wxString GetWindowMenuLabelTranslated() const; + // Retrieve the current window menu label: it can be different from + // "Window" when using non-English translations and can also be different + // from wxGetTranslation("Window") if the locale has changed since the + // "Window" menu was added. + const wxString& MSWGetCurrentWindowMenuLabel() const + { return m_currentWindowMenuLabel; } // handlers // -------- @@ -157,7 +158,7 @@ private: bool m_activationNotHandled; // holds the current translation for the window menu label - wxString m_windowMenuLabelTranslated; + wxString m_currentWindowMenuLabel; friend class WXDLLIMPEXP_FWD_CORE wxMDIChildFrame; diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index 7ddd464dfe..6870069d38 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -75,9 +75,6 @@ const int wxLAST_MDI_CHILD = wxFIRST_MDI_CHILD + 8; // child. const int wxID_MDI_MORE_WINDOWS = wxLAST_MDI_CHILD + 1; -// The MDI "Window" menu label -const char *WINDOW_MENU_LABEL = gettext_noop("&Window"); - // --------------------------------------------------------------------------- // private functions // --------------------------------------------------------------------------- @@ -277,16 +274,6 @@ int wxMDIParentFrame::GetChildFramesCount() const return count; } -void wxMDIParentFrame::SetWindowMenuLabelTranslated() -{ - m_windowMenuLabelTranslated = wxGetTranslation(WINDOW_MENU_LABEL); -} - -const wxString wxMDIParentFrame::GetWindowMenuLabelTranslated() const -{ - return m_windowMenuLabelTranslated; -} - #if wxUSE_MENUS void wxMDIParentFrame::AddMDIChild(wxMDIChildFrame * WXUNUSED(child)) @@ -344,8 +331,12 @@ void wxMDIParentFrame::AddWindowMenu() // attach it to the menu bar. m_windowMenu->Attach(GetMenuBar()); - SetWindowMenuLabelTranslated(); - MDIInsertWindowMenu(GetClientWindow(), m_hMenu, GetMDIWindowMenu(this), GetWindowMenuLabelTranslated()); + // Store the current translation, we can't use _("Window") later in + // case the locale changes. + m_currentWindowMenuLabel = _("&Window"); + + MDIInsertWindowMenu(GetClientWindow(), m_hMenu, GetMDIWindowMenu(this), + m_currentWindowMenuLabel); } } @@ -353,7 +344,8 @@ void wxMDIParentFrame::RemoveWindowMenu() { if ( m_windowMenu ) { - MDIRemoveWindowMenu(GetClientWindow(), m_hMenu, GetWindowMenuLabelTranslated()); + MDIRemoveWindowMenu(GetClientWindow(), m_hMenu, + m_currentWindowMenuLabel); m_windowMenu->Detach(); } @@ -937,7 +929,7 @@ wxMDIChildFrame::~wxMDIChildFrame() DestroyChildren(); - MDIRemoveWindowMenu(NULL, m_hMenu, parent->GetWindowMenuLabelTranslated()); + MDIRemoveWindowMenu(NULL, m_hMenu, parent->MSWGetCurrentWindowMenuLabel()); // MDIRemoveWindowMenu() doesn't update the MDI menu when called with NULL // window, so do it ourselves. @@ -1058,14 +1050,15 @@ void wxMDIChildFrame::InternalSetMenuBar() wxMDIParentFrame * const parent = GetMDIParent(); MDIInsertWindowMenu(parent->GetClientWindow(), - m_hMenu, GetMDIWindowMenu(parent), parent->GetWindowMenuLabelTranslated()); + m_hMenu, GetMDIWindowMenu(parent), + parent->MSWGetCurrentWindowMenuLabel()); } void wxMDIChildFrame::DetachMenuBar() { wxMDIParentFrame * const parent = GetMDIParent(); - - MDIRemoveWindowMenu(NULL, m_hMenu, parent->GetWindowMenuLabelTranslated()); + + MDIRemoveWindowMenu(NULL, m_hMenu, parent->MSWGetCurrentWindowMenuLabel()); wxFrame::DetachMenuBar(); }