From 9d92ba185c1188c0d89797dac7a9af222bb456ca Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 24 Jul 2019 11:19:29 +0200 Subject: [PATCH] Fix truncating CJK mnemonics in control labels 13068d36032fea5028169e523b2eae7eb60006cb introduced code for stripping CJK mnemonics (i.e. trailing " (&X)") from the menu items, but due to the use of wxStripMenuCodes() in wxControl::GetLabelText(), it also applied to the control labels, resulting in wrongly measuring their size (because the text used for measurement didn't include the "(&X)" part) and truncating them in display. Fix this by adding an explicit wxStrip_CJKMnemonics and using it only in the code dealing with the menu item labels. This requires more changes than just modifying GetLabelText() to use some wxStrip_NoCJKMnemonics flag, but is more compatible as it's not impossible that some existing code using wxStripMenuCodes() could be broken by this change too, while now the existing behaviour is preserved. Also improve wxStrip_XXX documentation. Closes https://github.com/wxWidgets/wxWidgets/pull/1439 See #16736. Closes #18452. --- include/wx/utils.h | 14 ++++++++++-- interface/wx/utils.h | 48 ++++++++++++++++++++++++++++++++++++---- src/common/menucmn.cpp | 2 +- src/common/utilscmn.cpp | 2 +- src/gtk1/menu.cpp | 4 ++-- src/motif/menu.cpp | 8 +++---- src/motif/menuitem.cpp | 6 ++--- src/msw/mdi.cpp | 2 +- src/osx/cocoa/menu.mm | 4 ++-- src/osx/menu_osx.cpp | 4 ++-- src/osx/menuitem_osx.cpp | 6 ++--- tests/menu/menu.cpp | 10 ++++----- 12 files changed, 80 insertions(+), 30 deletions(-) diff --git a/include/wx/utils.h b/include/wx/utils.h index 037bb88582..099530b3d0 100644 --- a/include/wx/utils.h +++ b/include/wx/utils.h @@ -652,8 +652,18 @@ enum // strip everything after '\t' wxStrip_Accel = 2, - // strip everything (this is the default) - wxStrip_All = wxStrip_Mnemonics | wxStrip_Accel + // strip mnemonics of the form "(&X)" appended to the string (used in CJK + // translations) + wxStrip_CJKMnemonics = 4, + + // strip everything (this doesn't include wxStrip_CJKMnemonics for + // compatibility) + wxStrip_All = wxStrip_Mnemonics | wxStrip_Accel, + + // strip everything including CJK mnemonics, suitable for menu items labels + // only (despite its name, wxStripMenuCodes() is currently used for control + // labels too) + wxStrip_Menu = wxStrip_All | wxStrip_CJKMnemonics }; // strip mnemonics and/or accelerators from the label diff --git a/interface/wx/utils.h b/interface/wx/utils.h index b4267d0f92..da9a307f22 100644 --- a/interface/wx/utils.h +++ b/interface/wx/utils.h @@ -682,14 +682,54 @@ void wxSetDisplayName(const wxString& displayName); */ enum { - // strip '&' characters + /** + Strip '&' characters. + + This flag removes all the ampersands before another character and + replaces double ampersands with a single one. + */ wxStrip_Mnemonics = 1, - // strip everything after '\t' + /** + Strip everything after '\t'. + + This flags removes everything following the last TAB character in the + string, if any. + */ wxStrip_Accel = 2, - // strip everything (this is the default) - wxStrip_All = wxStrip_Mnemonics | wxStrip_Accel + /** + Strip everything looking like CJK mnemonic. + + CJK (Chinese, Japanese, Korean) translations sometimes preserve the + original English accelerator or mnemonic in the translated string by + putting it after the translated string in parentheses, e.g. the string + "&File" could be translated as " (&F)". + + This flag strips trailing "(&X)" from the string. + + @since 3.1.3 + */ + wxStrip_CJKMnemonics = 4, + + /** + Strip both mnemonics and accelerators. + + This is the value used by wxStripMenuCodes() by default. + + Note that, despite the name, this flag does @e not strip all, as it + doesn't include wxStrip_CJKMnemonics for compatibility. + */ + wxStrip_All = wxStrip_Mnemonics | wxStrip_Accel, + + /** + Strip everything from menu item labels. + + This flag is used by wxWidgets internally and removes CJK mnemonics + from the labels too, in addition to the usual mnemonics and + accelerators. It is only suitable for use with the menu items. + */ + wxStrip_Menu = wxStrip_All | wxStrip_CJKMnemonics }; /** diff --git a/src/common/menucmn.cpp b/src/common/menucmn.cpp index e62a8b3c3d..d665f18c20 100644 --- a/src/common/menucmn.cpp +++ b/src/common/menucmn.cpp @@ -326,7 +326,7 @@ void wxMenuItemBase::SetHelp(const wxString& str) wxString wxMenuItemBase::GetLabelText(const wxString& text) { - return wxStripMenuCodes(text); + return wxStripMenuCodes(text, wxStrip_Menu); } #if WXWIN_COMPATIBILITY_2_8 diff --git a/src/common/utilscmn.cpp b/src/common/utilscmn.cpp index 2206f3d895..cc35d05bd2 100644 --- a/src/common/utilscmn.cpp +++ b/src/common/utilscmn.cpp @@ -1177,7 +1177,7 @@ wxString wxStripMenuCodes(const wxString& in, int flags) // In some East Asian languages _("&File") translates as "(&F)" // Check for this first, otherwise fall through to the standard situation - if (flags & wxStrip_Mnemonics) + if ( flags & wxStrip_CJKMnemonics ) { wxString label(in), accel; int pos = in.Find('\t'); diff --git a/src/gtk1/menu.cpp b/src/gtk1/menu.cpp index 16e1d9d203..a85cfe5a72 100644 --- a/src/gtk1/menu.cpp +++ b/src/gtk1/menu.cpp @@ -756,9 +756,9 @@ void wxMenuItem::SetItemLabel( const wxString& string ) // Some optimization to avoid flicker wxString oldLabel = m_text; - oldLabel = wxStripMenuCodes(oldLabel); + oldLabel = wxStripMenuCodes(oldLabel, wxStrip_Menu); oldLabel.Replace(wxT("_"), wxEmptyString); - wxString label1 = wxStripMenuCodes(str); + wxString label1 = wxStripMenuCodes(str, wxStrip_Menu); wxString oldhotkey = GetHotKey(); // Store the old hotkey in Ctrl-foo format wxCharBuffer oldbuf = wxGTK_CONV( GetGtkHotKey(*this) ); // and as foo diff --git a/src/motif/menu.cpp b/src/motif/menu.cpp index cecd53cf3c..164a0361da 100644 --- a/src/motif/menu.cpp +++ b/src/motif/menu.cpp @@ -292,12 +292,12 @@ wxMenu *wxMenuBar::Remove(size_t pos) // Returns -1 if none found. int wxMenuBar::FindMenuItem(const wxString& menuString, const wxString& itemString) const { - const wxString stripped = wxStripMenuCodes(menuString); + const wxString stripped = wxStripMenuCodes(menuString, wxStrip_Menu); size_t menuCount = GetMenuCount(); for (size_t i = 0; i < menuCount; i++) { - if ( wxStripMenuCodes(m_titles[i]) == stripped ) + if ( wxStripMenuCodes(m_titles[i], wxStrip_Menu) == stripped ) return m_menus.Item(i)->GetData()->FindItem (itemString); } return wxNOT_FOUND; @@ -347,7 +347,7 @@ bool wxMenuBar::CreateMenuBar(wxFrame* parent) wxString title(m_titles[i]); menu->SetButtonWidget(menu->CreateMenu (this, menuBarW, menu, i, title, true)); - if (strcmp (wxStripMenuCodes(title), "Help") == 0) + if (strcmp (wxStripMenuCodes(title, wxStrip_Menu), "Help") == 0) XtVaSetValues ((Widget) menuBarW, XmNmenuHelpWidget, (Widget) menu->GetButtonWidget(), NULL); // tear off menu support @@ -478,7 +478,7 @@ WXWidget wxMenu::CreateMenu (wxMenuBar * menuBar, char mnem = wxFindMnemonic (title); menu = XmCreatePulldownMenu ((Widget) parent, wxMOTIF_STR("pulldown"), args, 3); - wxString title2(wxStripMenuCodes(title)); + wxString title2(wxStripMenuCodes(title, wxStrip_Menu)); wxXmString label_str(title2); buttonWidget = XtVaCreateManagedWidget(title2, #if wxUSE_GADGETS diff --git a/src/motif/menuitem.cpp b/src/motif/menuitem.cpp index afd2d75675..88b9c79378 100644 --- a/src/motif/menuitem.cpp +++ b/src/motif/menuitem.cpp @@ -161,7 +161,7 @@ void wxMenuItem::CreateItem (WXWidget menu, wxMenuBar * menuBar, { // Id=-3 identifies a Title item. m_buttonWidget = (WXWidget) XtVaCreateManagedWidget - (wxStripMenuCodes(m_text), + (wxStripMenuCodes(m_text, wxStrip_Menu), xmLabelGadgetClass, (Widget) menu, NULL); } else if (!IsSeparator() && !m_subMenu) @@ -174,7 +174,7 @@ void wxMenuItem::CreateItem (WXWidget menu, wxMenuBar * menuBar, txt = wxGetStockLabel(GetId(), wxSTOCK_WITH_ACCELERATOR|wxSTOCK_WITH_MNEMONIC); } - wxString strName = wxStripMenuCodes(txt); + wxString strName = wxStripMenuCodes(txt, wxStrip_Menu); if (IsCheckable()) { m_buttonWidget = (WXWidget) XtVaCreateManagedWidget (strName, @@ -302,7 +302,7 @@ void wxMenuItem::DestroyItem(bool full) void wxMenuItem::SetItemLabel(const wxString& label) { char mnem = wxFindMnemonic (label); - wxString label2 = wxStripMenuCodes(label); + wxString label2 = wxStripMenuCodes(label, wxStrip_Menu); m_text = label; diff --git a/src/msw/mdi.cpp b/src/msw/mdi.cpp index 98e819ec92..3d555e24cf 100644 --- a/src/msw/mdi.cpp +++ b/src/msw/mdi.cpp @@ -1570,7 +1570,7 @@ void MDIInsertWindowMenu(wxWindow *win, WXHMENU hMenu, HMENU menuWin, const wxSt MenuIterator it(hmenu); while ( it.GetNext(buf) ) { - const wxString label = wxStripMenuCodes(buf); + const wxString label = wxStripMenuCodes(buf, wxStrip_Menu); if ( label == wxGetStockLabel(wxID_HELP, wxSTOCK_NOFLAGS) ) { inserted = true; diff --git a/src/osx/cocoa/menu.mm b/src/osx/cocoa/menu.mm index 8c54ad4a53..0f600ea2c4 100644 --- a/src/osx/cocoa/menu.mm +++ b/src/osx/cocoa/menu.mm @@ -232,8 +232,8 @@ public : virtual NSMenu* MacCreateOrFindWindowMenu() { - NSString* nsWindowMenuTitle = wxNSStringWithWxString(wxStripMenuCodes(wxApp::s_macWindowMenuTitleName)); - NSString* nsAlternateWindowMenuTitle = wxNSStringWithWxString(wxStripMenuCodes(_("&Window"))); + NSString* nsWindowMenuTitle = wxNSStringWithWxString(wxStripMenuCodes(wxApp::s_macWindowMenuTitleName, wxStrip_Menu)); + NSString* nsAlternateWindowMenuTitle = wxNSStringWithWxString(wxStripMenuCodes(_("&Window"), wxStrip_Menu)); NSMenu* windowMenu = nil; diff --git a/src/osx/menu_osx.cpp b/src/osx/menu_osx.cpp index 60b3e466a3..e24ce81579 100644 --- a/src/osx/menu_osx.cpp +++ b/src/osx/menu_osx.cpp @@ -64,7 +64,7 @@ void wxMenu::Init() m_noEventsMode = false; m_radioData = NULL; - m_peer = wxMenuImpl::Create( this, wxStripMenuCodes(m_title) ); + m_peer = wxMenuImpl::Create( this, wxStripMenuCodes(m_title, wxStrip_Menu) ); // if we have a title, insert it in the beginning of the menu @@ -221,7 +221,7 @@ wxMenuItem *wxMenu::DoRemove(wxMenuItem *item) void wxMenu::SetTitle(const wxString& label) { m_title = label ; - GetPeer()->SetTitle( wxStripMenuCodes( label ) ); + GetPeer()->SetTitle( wxStripMenuCodes( label, wxStrip_Menu ) ); } bool wxMenu::ProcessCommand(wxCommandEvent & event) diff --git a/src/osx/menuitem_osx.cpp b/src/osx/menuitem_osx.cpp index 01170b14c0..80dc0a8f9a 100644 --- a/src/osx/menuitem_osx.cpp +++ b/src/osx/menuitem_osx.cpp @@ -41,10 +41,10 @@ wxMenuItem::wxMenuItem(wxMenu *pParentMenu, // In other languages there is no difference in naming the Exit/Quit menu item between MacOS and Windows guidelines // therefore these item must not be translated if (pParentMenu != NULL && !pParentMenu->GetNoEventsMode()) - if ( wxStripMenuCodes(m_text).Upper() == wxT("EXIT") ) + if ( wxStripMenuCodes(m_text, wxStrip_Menu).Upper() == wxT("EXIT") ) m_text = wxT("Quit\tCtrl+Q") ; - wxString text = wxStripMenuCodes(m_text, (pParentMenu != NULL && pParentMenu->GetNoEventsMode()) ? wxStrip_Accel : wxStrip_All); + wxString text = wxStripMenuCodes(m_text, (pParentMenu != NULL && pParentMenu->GetNoEventsMode()) ? wxStrip_Accel : wxStrip_Menu); if (text.IsEmpty() && !IsSeparator()) { wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?")); @@ -191,7 +191,7 @@ void wxMenuItem::UpdateItemText() if ( !m_parentMenu ) return ; - wxString text = wxStripMenuCodes(m_text, m_parentMenu != NULL && m_parentMenu->GetNoEventsMode() ? wxStrip_Accel : wxStrip_All); + wxString text = wxStripMenuCodes(m_text, m_parentMenu != NULL && m_parentMenu->GetNoEventsMode() ? wxStrip_Accel : wxStrip_Menu); if (text.IsEmpty() && !IsSeparator()) { wxASSERT_MSG(wxIsStockID(GetId()), wxT("A non-stock menu item with an empty label?")); diff --git a/tests/menu/menu.cpp b/tests/menu/menu.cpp index 6e6cd6f36b..c31a338e93 100644 --- a/tests/menu/menu.cpp +++ b/tests/menu/menu.cpp @@ -365,8 +365,8 @@ void MenuTestCase::TranslatedMnemonics() wxString filemenu = m_frame->GetMenuBar()->GetMenuLabel(0); CPPUNIT_ASSERT_EQUAL ( - wxStripMenuCodes(GetTranslatedString(trans, "&File")), - wxStripMenuCodes(GetTranslatedString(trans, filemenu)) + wxStripMenuCodes(GetTranslatedString(trans, "&File"), wxStrip_Menu), + wxStripMenuCodes(GetTranslatedString(trans, filemenu), wxStrip_Menu) ); // Test strings that have shortcuts. Duplicate non-mnemonic translations @@ -374,21 +374,21 @@ void MenuTestCase::TranslatedMnemonics() CPPUNIT_ASSERT_EQUAL ( GetTranslatedString(trans, "Edit"), - wxStripMenuCodes(GetTranslatedString(trans, "E&dit\tCtrl+E")) + wxStripMenuCodes(GetTranslatedString(trans, "E&dit\tCtrl+E"), wxStrip_Menu) ); // "Vie&w" also has a space before the (&W) CPPUNIT_ASSERT_EQUAL ( GetTranslatedString(trans, "View"), - wxStripMenuCodes(GetTranslatedString(trans, "Vie&w\tCtrl+V")) + wxStripMenuCodes(GetTranslatedString(trans, "Vie&w\tCtrl+V"), wxStrip_Menu) ); // Test a 'normal' mnemonic too: the translation is "Preten&d" CPPUNIT_ASSERT_EQUAL ( "Pretend", - wxStripMenuCodes(GetTranslatedString(trans, "B&ogus")) + wxStripMenuCodes(GetTranslatedString(trans, "B&ogus"), wxStrip_Menu) ); } #endif // wxUSE_INTL