wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
6.2k stars 1.78k forks source link

Documentation 'Translating Menu Accelerators' is outdated (for master) #24323

Open taler21 opened 9 months ago

taler21 commented 9 months ago

The documentation states:

https://github.com/wxWidgets/wxWidgets/blob/47b011c665b7e07eef90489c22ff426a60b38ddb/docs/doxygen/overviews/internationalization.h#L114-L118

But with the current master it is no longer sufficient to only provide translations of "ctrl", "alt" and "shift".

Since 4dcda62f9d9789519b6dd7629a8a256641dc56ad (Normalize displayed accelerators in wxMSW too, 2022-12-09) it is also necessary to provide translations of "Ctrl+", "Alt+" and "Shift+". Otherwise, with wxMSW, the modifier keys will be displayed normalized in English.

Platform and version information

vadz commented 9 months ago

I'm not sure if we did this intentionally, i.e. can "Ctrl+" be actually translated to something different from the translation of "Ctrl" followed by "+" in any language?

taler21 commented 9 months ago

Parsing the accelerator string is done with hardcoded '+' and '-'.

https://github.com/wxWidgets/wxWidgets/blob/47b011c665b7e07eef90489c22ff426a60b38ddb/src/common/accelcmn.cpp#L181-L191

vadz commented 9 months ago

In principle, translations could map everything to Accel-Letter if the target language preferred this convention to using +. I have no idea if this really exists though.

taler21 commented 9 months ago

You mean translating '+' separately in a context called Accel-Letter?

I took a look to the existing wxWidgets catalogs.

In any case there will be an incompatibility from wxWidgets 3.2 to 3.3.

Sure, not everyone will be affected by this incompatibility. Those using English texts in their source code and loading the standard wxWidgets catalogs will not be affected. But those who use non-English texts in their source code and/or do not load the standard wxWidgets catalogs are affected and will need to adapt their provided translations.

vadz commented 9 months ago

Thanks for checking this. If there are no languages that need to translate Ctrl+ to Ctrl-, then I agree that it would be better to revert to 3.2 behaviour and just translate the modifiers separately.

I'm not sure what would be the minimal change required to do it, but if you do, please let me know and/or just make the PR with it directly. TIA!

taler21 commented 9 months ago

Sorry, but I don't understand how "RawCtrl" is supposed to work.

The documentation states:

https://github.com/wxWidgets/wxWidgets/blob/f24b3d54833a1c15ae3bba87dead83bf9a90c41d/interface/wx/menuitem.h#L458-L463

  1. When parsing, "Ctrl" and "RawCtrl" are treated as different keys on each platform. https://github.com/wxWidgets/wxWidgets/blob/f24b3d54833a1c15ae3bba87dead83bf9a90c41d/src/common/accelcmn.cpp#L192-L193 But in wxAcceleratorEntry::ToString() "RawCtrl+" is only translated for __WXMAC__. https://github.com/wxWidgets/wxWidgets/blob/f24b3d54833a1c15ae3bba87dead83bf9a90c41d/src/common/accelcmn.cpp#L348-L351
  2. How should translators translate the lower case name of the modifier "rawctrl"? Should it even be translated?
  3. How should translators translate this modifier in the notation that is displayed ("RawCtrl+")? Should it be the same translation as of "Ctrl+"?
  4. If so, wouldn't it be better to map "RawCtrl+" to "Ctrl+" and only translate the latter?
  5. And how else would "RawCtrl+" be translated in English to "Ctrl+", without a standard English wxWidgets catalog?
taler21 commented 9 months ago

Another aspect of using wxAcceleratorEntry::ToString() to display normalized accelerators in the menu is that the modifiers are not in the usual order.

https://github.com/wxWidgets/wxWidgets/blob/f24b3d54833a1c15ae3bba87dead83bf9a90c41d/src/common/accelcmn.cpp#L342-L351

The usual order of modifiers under MSW is [Ctrl+][Shift+][Alt+], but definitely not [Alt+] in the first place.

Don't know if this can be easily changed since this code is also called by wxAcceleratorEntry::ToRawString() to generate a textual representation of the accelerator for saving in configuration files. Maybe yes, since such loaded text is probably parsed again by wxAcceleratorEntry::FromString() anyway and the order of the modifiers doesn't matter there.

vadz commented 1 week ago

I have to admit that I'm completely lost here because I don't understand how is this supposed to work at all. Looking at the internat sample, it has "&Test locale availability...\tCtrl-T" menu command and e.g. German translations just translates it entirely (and IMO wrongly because it keeps "Ctrl" instead of using "Strg" as it should, AFAIK). And if I provide the translation for just "&Test locale availability...", it is not used at all, at least in wxGTK, and no lookup for Ctrl- or anything like this, is done at all.

What am I missing?

taler21 commented 1 week ago

You are right, the German translation is incorrect. It should be as follows, and in my opinion also be corrected in 3.2.

diff --git "a/samples/internat/de/internat.po" "b/samples/internat/de/internat.po"
index 53e61c5582..7cc61ae692 100644
--- "a/samples/internat/de/internat.po"
+++ "b/samples/internat/de/internat.po"
@@ -39,7 +39,7 @@ msgstr "Internationale wxWidgets-Anwendung"

 #: internat.cpp:295
 msgid "&Test locale availability...\tCtrl-T"
-msgstr "&Teste Gebietsschemaverfügbarkeit...\tCtrl-T"
+msgstr "&Teste Gebietsschemaverfügbarkeit...\tStrg-T"

 #: internat.cpp:300
 msgid "E&xit"

As far as I know, wxGTK behaves differently than wxMSW in this regard. In wxGTK, how modifiers are displayed depends on the system language. When catalogs are loaded via wxTranslations in a language other than the system language, the modifiers are not displayed as translated in the catalogs, but according to the system language.

Unfortunately, loading catalogs in any language can no longer be tested in the internat sample since eb5bffd3caee3225f64bef56bb6b6e8147e81887 (Stop allowing to select the language in the internat sample, 2021-08-14).

vadz commented 1 week ago

Unfortunately, loading catalogs in any language can no longer be tested in the internat sample since eb5bffd (Stop allowing to select the language in the internat sample, 2021-08-14).

They can be still tested under Linux, by setting LC_ALL=de_DE before running the sample, but not under Windows, unfortunately.