From c485f44ba0338ee51c6e5b09f958e76a776de332 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 31 May 2015 18:00:44 +0200 Subject: [PATCH] Use GetMenuItemInfo() instead of GetMenuString() in wxMSW MDI code. This allows to detect errors with retrieving the item label more reliably as GetMenuItemInfo() sets the last error correctly while GetMenuString() doesn't seem to always do it and some users report apparently bogus debug messages about its failure. --- src/msw/mdi.cpp | 90 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 28 deletions(-) diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index 5ad9d71491..c742e20c36 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -1514,6 +1514,59 @@ void MDISetMenu(wxWindow *win, HMENU hmenuFrame, HMENU hmenuWindow) ::DrawMenuBar(GetWinHwnd(parent)); } +class MenuIterator +{ +public: + explicit MenuIterator(HMENU hmenu) + : m_hmenu(hmenu), + m_numItems(::GetMenuItemCount(hmenu)), + m_pos(-1) + { + m_mii.fMask = MIIM_STRING; + m_mii.dwTypeData = m_buf; + } + + bool GetNext(wxString& str) + { + // Loop until we get the label of the next menu item. + for ( m_pos++; m_pos < m_numItems; m_pos++ ) + { + // As cch field is updated by GetMenuItemInfo(), it's important to + // reset it to the size of the buffer before each call. + m_mii.cch = WXSIZEOF(m_buf); + + if ( !::GetMenuItemInfo(m_hmenu, m_pos, TRUE, &m_mii) ) + { + wxLogLastError(wxString::Format("GetMenuItemInfo(%d)", m_pos)); + continue; + } + + if ( !m_mii.cch ) + { + // This isn't a string menu at all. + continue; + } + + str = m_buf; + return true; + } + + return false; + } + + int GetPos() const { return m_pos; } + +private: + const HMENU m_hmenu; + const int m_numItems; + int m_pos; + + wxChar m_buf[1024]; + WinStruct m_mii; + + wxDECLARE_NO_COPY_CLASS(MenuIterator); +}; + void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) { HMENU hmenu = (HMENU)hMenu; @@ -1521,23 +1574,17 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin) if ( menuWin ) { // Try to insert Window menu in front of Help, otherwise append it. - int N = GetMenuItemCount(hmenu); bool inserted = false; - for ( int i = 0; i < N; i++ ) + wxString buf; + MenuIterator it(hmenu); + while ( it.GetNext(buf) ) { - wxChar buf[256]; - if ( !::GetMenuString(hmenu, i, buf, WXSIZEOF(buf), MF_BYPOSITION) ) - { - wxLogLastError(wxT("GetMenuString")); - - continue; - } - const wxString label = wxStripMenuCodes(buf); if ( label == wxGetStockLabel(wxID_HELP, wxSTOCK_NOFLAGS) ) { inserted = true; - ::InsertMenu(hmenu, i, MF_BYPOSITION | MF_POPUP | MF_STRING, + ::InsertMenu(hmenu, it.GetPos(), + MF_BYPOSITION | MF_POPUP | MF_STRING, (UINT_PTR)menuWin, wxString(wxGetTranslation(WINDOW_MENU_LABEL)).t_str()); break; @@ -1561,26 +1608,13 @@ void MDIRemoveWindowMenu(wxWindow *win, WXHMENU hMenu) if ( hmenu ) { - wxChar buf[1024]; - - int N = ::GetMenuItemCount(hmenu); - for ( int i = 0; i < N; i++ ) + wxString buf; + MenuIterator it(hmenu); + while ( it.GetNext(buf) ) { - if ( !::GetMenuString(hmenu, i, buf, WXSIZEOF(buf), MF_BYPOSITION) ) - { - // Ignore successful read of menu string with length 0 which - // occurs, for example, for a maximized MDI child system menu - if ( ::GetLastError() != 0 ) - { - wxLogLastError(wxT("GetMenuString")); - } - - continue; - } - if ( wxStrcmp(buf, wxGetTranslation(WINDOW_MENU_LABEL)) == 0 ) { - if ( !::RemoveMenu(hmenu, i, MF_BYPOSITION) ) + if ( !::RemoveMenu(hmenu, it.GetPos(), MF_BYPOSITION) ) { wxLogLastError(wxT("RemoveMenu")); }