Avoid having Bing mis-translate native UI language name back as "English"

CHANGELOG.md

@@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Find TMX files in `Generated` and `User Modified` directories
 
+- Don't ask Bing translator to translate language names: https://github.com/sillsdev/l10nsharp/issues/66.
+  Also don't display the name a second time in parentheses if English and native name are identical.
+
 ## [4.0.0] - 2019-05-16
 
 ### Changed

src/L10NSharp/L10NSharp-Designer.csproj

@@ -123,6 +123,7 @@
     <Compile Include="UI\LanguageChoosingDialog.Designer.cs">
       <DependentUpon>LanguageChoosingDialog.cs</DependentUpon>
     </Compile>
+    <Compile Include="UI\LanguageChoosingDialogViewModel.cs" />
     <Compile Include="UI\LanguageChoosingSimpleDialog.cs">
       <SubType>Form</SubType>
     </Compile>

src/L10NSharp/UI/LanguageChoosingDialog.cs

@@ -1,47 +1,29 @@
 using System;
 using System.Drawing;
-using System.Globalization;
 using System.Windows.Forms;
 using L10NSharp.Translators;
 
 namespace L10NSharp.UI
 {
 	public partial class LanguageChoosingDialog : Form
 	{
-		private readonly L10NCultureInfo _requestedCulture;
-		private string _originalMessageTemplate;
+		private readonly LanguageChoosingDialogViewModel _model;
 
 		public LanguageChoosingDialog(L10NCultureInfo requestedCulture, Icon icon)
 		{
-			_requestedCulture = requestedCulture;
 			InitializeComponent();
 			this.Icon = icon;
-			_originalMessageTemplate = _messageLabel.Text;
-			_messageLabel.Text = string.Format(_originalMessageTemplate, requestedCulture.EnglishName, requestedCulture.NativeName);
-			Application.Idle += new EventHandler(Application_Idle);
+			_model = new LanguageChoosingDialogViewModel(_messageLabel.Text, _OKButton.Text, Text, requestedCulture, () => { Application.Idle += Application_Idle; } );
+			_messageLabel.Text = _model.Message;
 		}
 
 		void Application_Idle(object sender, EventArgs e)
 		{
-			Application.Idle -= new EventHandler(Application_Idle);
-			var translator = new BingTranslator("en", _requestedCulture.TwoLetterISOLanguageName);
-			try
-			{
-				var s = translator.TranslateText(string.Format(_originalMessageTemplate, _requestedCulture.EnglishName, _requestedCulture.NativeName));
-				if (!string.IsNullOrEmpty(s))
-				{
-					_messageLabel.Text = s;
-					// In general, we will be able to translate OK and the title bar text iff we were able to translate
-					// the message.  This assumption saves a few processor cycles and prevents disappearing text when
-					// a language has not been localized (as is likely the case when we display this dialog).
-					_OKButton.Text = translator.TranslateText("OK");
-					Text = translator.TranslateText(Text);
-				}
-			}
-			catch (Exception)
-			{
-				//swallow
-			}
+			Application.Idle -= Application_Idle;
+			_model.SetTranslator(new BingTranslator("en", _model.RequestedCultureTwoLetterISOLanguageName));
+			_messageLabel.Text = _model.Message;
+			_OKButton.Text = _model.AcceptButtonText;
+			Text = _model.WindowTitle;
 		}
 
 		public string SelectedLanguage;

src/L10NSharp/UI/LanguageChoosingDialogViewModel.cs

@@ -0,0 +1,70 @@
+using System;
+using L10NSharp.Translators;
+
+namespace L10NSharp.UI
+{
+	public class LanguageChoosingDialogViewModel
+	{
+		private readonly L10NCultureInfo _requestedCulture;
+		private readonly string _messageLabelFormat;
+		private readonly string _acceptButtonText;
+		private readonly string _windowTitle;
+
+		public LanguageChoosingDialogViewModel(string messageLabelFormat, string acceptButtonText, string windowTitle,
+			L10NCultureInfo requestedCulture, Action nonEnglishUiAction)
+		{
+			_messageLabelFormat = messageLabelFormat;
+			AcceptButtonText = _acceptButtonText = acceptButtonText;
+			WindowTitle = _windowTitle = windowTitle;
+			_requestedCulture = requestedCulture;
+			if (requestedCulture.EnglishName == requestedCulture.NativeName)
+			{
+				// It looks weird and stupid to display "English (English)" or any other such pair where the two strings are the same.
+				_messageLabelFormat = _messageLabelFormat.Replace(" ({1})", "");
+			}
+			Message = string.Format(_messageLabelFormat, requestedCulture.EnglishName, requestedCulture.NativeName);
+			if (requestedCulture.TwoLetterISOLanguageName != "en")
+				nonEnglishUiAction?.Invoke();
+		}
+
+		public void SetTranslator(TranslatorBase translator)
+		{
+			try
+			{
+				var s = translator.TranslateText(string.Format(_messageLabelFormat, _requestedCulture.EnglishName, "{0}"));
+				if (s.Contains("{0}") && s.Length > 5) // If we just get back "{0} or "({0})", we won't consider that useful.
+				{
+					// Bing will presumably have translated the English string into the native language, so now we want
+					// to display the English name in parentheses. (As a sanity check, we could look to see whether the
+					// native name is in the string, but there could be situations where it may not be an exact match.)
+					s = string.Format(s, _requestedCulture.EnglishName);
+				}
+				else if (_messageLabelFormat.Contains("{1}"))
+				{
+					// If we already weeded out the param (because the language names are the same), there's no need to re-try (in case it's slow).
+					// This is just a fall-back in case there is some rare situation where the translator chokes on the presence of a formatting param in the string.
+					s = translator.TranslateText(string.Format(_messageLabelFormat, _requestedCulture.EnglishName, _requestedCulture.NativeName));
+				}
+
+				if (!string.IsNullOrEmpty(s))
+				{
+					Message = s;
+					// In general, we will be able to translate OK and the title bar text iff we were able to translate
+					// the message.  This assumption saves a few processor cycles and prevents disappearing text when
+					// a language has not been localized (as is likely the case when we display this dialog).
+					AcceptButtonText = translator.TranslateText(_acceptButtonText);
+					WindowTitle = translator.TranslateText(_windowTitle);
+				}
+			}
+			catch (Exception)
+			{
+				//swallow
+			}
+		}
+
+		public string RequestedCultureTwoLetterISOLanguageName => _requestedCulture.TwoLetterISOLanguageName;
+		public string Message { get; private set; }
+		public string AcceptButtonText { get; private set; }
+		public string WindowTitle { get; private set; }
+	}
+}

src/L10NSharpTests/LanguageChoosingDialogViewModelTests.cs

@@ -0,0 +1,213 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using L10NSharp.Translators;
+using L10NSharp.UI;
+using NUnit.Framework;
+
+namespace L10NSharp.Tests
+{
+	[TestFixture]
+	class LanguageChoosingDialogViewModelTests
+	{
+		[Test]
+		public void Constructor_RequestedCultureEnglish_MessageOnlyHasEnglishOnceAndNonEnglishActionIsNotRun()
+		{
+			var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"),
+				() => throw new Exception("Non-english UI Action should not have been called"));
+			Assert.AreEqual("Blah English", model.Message);
+			// We'll go ahead and confirm that the other properties come through as expected also.
+			Assert.AreEqual("en", model.RequestedCultureTwoLetterISOLanguageName);
+			Assert.AreEqual("OK", model.AcceptButtonText);
+			Assert.AreEqual("Choose a Language", model.WindowTitle);
+		}
+
+		[Test]
+		public void Constructor_RequestedCultureTypicalNonEnglish_MessageHasNativeNameAndEnglishNameAndNonEnglishActionIsRun()
+		{
+			var nonEnglishActionGotCalled = false;
+			var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"),
+				() => nonEnglishActionGotCalled = true);
+			Assert.AreEqual("Blah German (Deutsch)", model.Message);
+			Assert.IsTrue(nonEnglishActionGotCalled);
+			// We'll go ahead and confirm that the other properties come through as expected also.
+			Assert.AreEqual("de", model.RequestedCultureTwoLetterISOLanguageName);
+			Assert.AreEqual("OK", model.AcceptButtonText);
+			Assert.AreEqual("Choose a Language", model.WindowTitle);
+		}
+
+		[Test]
+		public void Constructor_RequestedCultureWithNativeNameEqualToEnglishName_MessageOnlyLanguageNameOnceAndNonEnglishActionIsRun()
+		{
+			var nonEnglishActionGotCalled = false;
+			var culture = L10NCultureInfo.GetCultures(CultureTypes.AllCultures).FirstOrDefault(c => c.EnglishName == c.NativeName);
+			Assume.That(culture != null);
+			var model = new LanguageChoosingDialogViewModel("Blah {0} ({1})", "OK", "Choose a Language",
+				culture,
+				() => nonEnglishActionGotCalled = true);
+			Assert.AreEqual($"Blah {culture.EnglishName}", model.Message);
+			Assert.IsTrue(nonEnglishActionGotCalled);
+			// We'll go ahead and confirm that the other properties come through as expected also.
+			Assert.AreEqual(culture.TwoLetterISOLanguageName, model.RequestedCultureTwoLetterISOLanguageName);
+			Assert.AreEqual("OK", model.AcceptButtonText);
+			Assert.AreEqual("Choose a Language", model.WindowTitle);
+		}
+
+		[Test]
+		public void Constructor_RequestedCultureEnglishFormatStringHasNoParam1_MessageHasEnglishSubstitutedAsAparam0()
+		{
+			var model = new LanguageChoosingDialogViewModel("Blah {0} yup", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"),
+				null);
+			Assert.AreEqual("Blah English yup", model.Message);
+		}
+
+		[TestCase("en")]
+		[TestCase("fr")]
+		public void Constructor_FormatStringHasNoParams_MessageEqualsFormatString(string languageCode)
+		{
+			var model = new LanguageChoosingDialogViewModel("No format parameters", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == languageCode),
+				null);
+			Assert.AreEqual("No format parameters", model.Message);
+		}
+
+		[TestCase("Blah {2}")]
+		[TestCase("Blah {-1}")]
+		[TestCase("Blah {a}")]
+		public void Constructor_BogusFormatString_Throws(string format)
+		{
+			Assert.Throws<FormatException>(() => new LanguageChoosingDialogViewModel(format, "OK", "Choose a Language",
+				L10NCultureInfo.CurrentCulture, null));
+		}
+
+		[Test]
+		public void SetTranslator_RequestedCultureEnglish_TranslationAppliedOnceToEachString()
+		{
+			var model = new LanguageChoosingDialogViewModel("Blah {0} ({1}) yup!", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "en"),
+				null);
+			Assert.AreEqual("Blah English yup!", model.Message);
+			var translator = new TestTranslatorBumpyFrog();
+			model.SetTranslator(translator);
+			Assert.AreEqual("Bumpy frog Blah English yup!", model.Message);
+			Assert.AreEqual("Bumpy frog OK", model.AcceptButtonText);
+			Assert.AreEqual("Bumpy frog Choose a Language", model.WindowTitle);
+			Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "Blah English yup!", "OK", "Choose a Language" }));
+		}
+
+		[Test]
+		public void SetTranslator_RequestedCultureGermanNormalTranslation_TranslationAppliedOnceToEachString()
+		{
+			var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"),
+				null);
+			Assert.AreEqual("No localization for German (Deutsch)", model.Message);
+			var translator = new TestTranslatorGerman();
+			model.SetTranslator(translator);
+			Assert.AreEqual("Ich spreche No localization for Deutsch (German)", model.Message);
+			Assert.AreEqual("Ich spreche OK", model.AcceptButtonText);
+			Assert.AreEqual("Ich spreche Choose a Language", model.WindowTitle);
+			Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for German ({0})", "OK", "Choose a Language" }));
+		}
+
+		[Test]
+		public void SetTranslator_ChangeTranslator_TranslationAppliedToOriginalString()
+		{
+			var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "de"),
+				null);
+			Assert.AreEqual("No localization for German (Deutsch)", model.Message);
+			model.SetTranslator(new TestTranslatorBumpyFrog());
+			model.SetTranslator(new TestTranslatorGerman());
+			Assert.AreEqual("Ich spreche No localization for Deutsch (German)", model.Message);
+			Assert.AreEqual("Ich spreche OK", model.AcceptButtonText);
+			Assert.AreEqual("Ich spreche Choose a Language", model.WindowTitle);
+		}
+
+		[Test]
+		public void SetTranslator_RequestedCultureSpanishChokesOnFormatParam_TranslationReappliedToStringWithoutParam()
+		{
+			var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "OK", "Choose a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "es"),
+				null);
+			Assert.AreEqual("No localization for Spanish (español)", model.Message);
+			var translator = new TestTranslatorSpanishChokesOnFormatParam();
+			model.SetTranslator(translator);
+			// Note: the test translator mimics Bing's behavior of replacing the English name of the requested language with the word "English" in the translation.
+			Assert.AreEqual("No choke No localization for English (español)", model.Message);
+			Assert.AreEqual("No choke OK", model.AcceptButtonText);
+			Assert.AreEqual("No choke Choose a Language", model.WindowTitle);
+			Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for Spanish ({0})", "No localization for Spanish (español)", "OK", "Choose a Language" }));
+		}
+
+		[Test]
+		public void SetTranslator_TranslatorThrowsException_ExceptionSwallowedAndNoAttemptToTranslateOtherStrings()
+		{
+			var model = new LanguageChoosingDialogViewModel("No localization for {0} ({1})", "Okey-dokey", "Select a Language",
+				L10NCultureInfo.GetCultures(CultureTypes.NeutralCultures).First(c => c.TwoLetterISOLanguageName == "es"),
+				null);
+			var translator = new TestTranslatorThrowsException();
+			model.SetTranslator(translator);
+			Assert.AreEqual("No localization for Spanish (español)", model.Message);
+			Assert.AreEqual("Okey-dokey", model.AcceptButtonText);
+			Assert.AreEqual("Select a Language", model.WindowTitle);
+			Assert.IsTrue(translator.SourceStrings.SequenceEqual(new[] { "No localization for Spanish ({0})" }));
+		}
+
+		private class TestTranslatorBase : TranslatorBase
+		{
+			public List<string> SourceStrings { get; }
+
+			public TestTranslatorBase()
+			{
+				SourceStrings = new List<string>();
+			}
+
+			protected override string InternalTranslate(string srcText)
+			{
+				SourceStrings.Add(srcText);
+				return srcText;
+			}
+		}
+
+		private class TestTranslatorBumpyFrog : TestTranslatorBase
+		{
+			protected override string InternalTranslate(string srcText)
+			{
+				return "Bumpy frog " + base.InternalTranslate(srcText);
+			}
+		}
+
+		private class TestTranslatorGerman : TestTranslatorBase
+		{
+			protected override string InternalTranslate(string srcText)
+			{
+				return "Ich spreche " + base.InternalTranslate(srcText).Replace("German", "Deutsch");
+			}
+		}
+
+		private class TestTranslatorSpanishChokesOnFormatParam : TestTranslatorBase
+		{
+			protected override string InternalTranslate(string srcText)
+			{
+				var s = base.InternalTranslate(srcText);
+				if (s.Contains("{0}"))
+					return "";
+				return "No choke " + s.Replace("Spanish", "English"); // Looks weird, but Bing actually does this!
+			}
+		}
+
+		private class TestTranslatorThrowsException : TestTranslatorBase
+		{
+			protected override string InternalTranslate(string srcText)
+			{
+				base.InternalTranslate(srcText);
+				throw new Exception("This should get swallowed");
+			}
+		}
+	}
+}