Remove LRE/LRO characters from results and error messages (microsoft#…

…1161) * Remove LRE/LRO characters and rely on Xaml to correctly displayed the numbers and error messages RtL * unit tests
UBC-CS · Apr 30, 2020 · 6e521d8 · 6e521d8
1 parent 2cafb0d
commit 6e521d8
Show file tree

Hide file tree

Showing 20 changed files with 127 additions and 188 deletions.
diff --git a/src/CalcManager/CalculatorHistory.cpp b/src/CalcManager/CalculatorHistory.cpp
@@ -45,12 +45,7 @@ unsigned int CalculatorHistory::AddToHistory(
 
     spHistoryItem->historyItemVector.spTokens = tokens;
     spHistoryItem->historyItemVector.spCommands = commands;
-
-    // to be changed when pszexp is back
-    wstring generatedExpression = GetGeneratedExpression(*tokens);
-    // Prefixing and suffixing the special Unicode markers to ensure that the expression
-    // in the history doesn't get broken for RTL languages
-    spHistoryItem->historyItemVector.expression = L'\u202d' + generatedExpression + L'\u202c';
+    spHistoryItem->historyItemVector.expression = GetGeneratedExpression(*tokens);
     spHistoryItem->historyItemVector.result = wstring(result);
     return AddItem(spHistoryItem);
 }

diff --git a/src/CalcViewModel/Common/Utils.h b/src/CalcViewModel/Common/Utils.h
@@ -241,9 +241,6 @@ namespace Utils
         };
     }
 
-    const wchar_t LRE = 0x202a; // Left-to-Right Embedding
-    const wchar_t PDF = 0x202c; // Pop Directional Formatting
-    const wchar_t LRO = 0x202d; // Left-to-Right Override
 
     // Regular DependencyProperty
     template <typename TOwner, typename TType>

diff --git a/src/CalcViewModel/StandardCalculatorViewModel.cpp b/src/CalcViewModel/StandardCalculatorViewModel.cpp
@@ -134,23 +134,14 @@ StandardCalculatorViewModel::StandardCalculatorViewModel()
     AreProgrammerRadixOperatorsEnabled = false;
 }
 
-String ^ StandardCalculatorViewModel::LocalizeDisplayValue(_In_ wstring const& displayValue, _In_ bool isError)
+String ^ StandardCalculatorViewModel::LocalizeDisplayValue(_In_ wstring const& displayValue)
 {
     wstring result(displayValue);
-
     LocalizationSettings::GetInstance().LocalizeDisplayValue(&result);
-
-    // WINBLUE: 440747 - In BiDi languages, error messages need to be wrapped in LRE/PDF
-    if (isError && m_isRtlLanguage)
-    {
-        result.insert(result.begin(), Utils::LRE);
-        result.push_back(Utils::PDF);
-    }
-
     return ref new Platform::String(result.c_str());
 }
 
-String ^ StandardCalculatorViewModel::CalculateNarratorDisplayValue(_In_ wstring const& displayValue, _In_ String ^ localizedDisplayValue, _In_ bool isError)
+String ^ StandardCalculatorViewModel::CalculateNarratorDisplayValue(_In_ wstring const& displayValue, _In_ String ^ localizedDisplayValue)
 {
     String ^ localizedValue = localizedDisplayValue;
     String ^ automationFormat = m_localizedCalculationResultAutomationFormat;
@@ -159,7 +150,7 @@ String ^ StandardCalculatorViewModel::CalculateNarratorDisplayValue(_In_ wstring
     if (Utils::IsLastCharacterTarget(displayValue, m_decimalSeparator))
     {
         // remove the decimal separator, to avoid a long pause between words
-        localizedValue = LocalizeDisplayValue(displayValue.substr(0, displayValue.length() - 1), isError);
+        localizedValue = LocalizeDisplayValue(displayValue.substr(0, displayValue.length() - 1));
 
         // Use a format which has a word in the decimal separator's place
         // "The Display is 10 point"
@@ -195,11 +186,11 @@ String ^ StandardCalculatorViewModel::GetNarratorStringReadRawNumbers(_In_ Strin
 
 void StandardCalculatorViewModel::SetPrimaryDisplay(_In_ String ^ displayStringValue, _In_ bool isError)
 {
-    String ^ localizedDisplayStringValue = LocalizeDisplayValue(displayStringValue->Data(), isError);
+    String ^ localizedDisplayStringValue = LocalizeDisplayValue(displayStringValue->Data());
 
     // Set this variable before the DisplayValue is modified, Otherwise the DisplayValue will
     // not match what the narrator is saying
-    m_CalculationResultAutomationName = CalculateNarratorDisplayValue(displayStringValue->Data(), localizedDisplayStringValue, isError);
+    m_CalculationResultAutomationName = CalculateNarratorDisplayValue(displayStringValue->Data(), localizedDisplayStringValue);
 
     AreAlwaysOnTopResultsUpdated = false;
     if (DisplayValue != localizedDisplayStringValue)
@@ -418,7 +409,7 @@ void StandardCalculatorViewModel::SetMemorizedNumbers(const vector<wstring>& new
             MemoryItemViewModel ^ memorySlot = ref new MemoryItemViewModel(this);
             memorySlot->Position = 0;
             localizer.LocalizeDisplayValue(&stringValue);
-            memorySlot->Value = Utils::LRO + ref new String(stringValue.c_str()) + Utils::PDF;
+            memorySlot->Value = ref new String(stringValue.c_str());
 
             MemorizedNumbers->InsertAt(0, memorySlot);
             IsMemoryEmpty = IsAlwaysOnTop;
@@ -440,7 +431,7 @@ void StandardCalculatorViewModel::SetMemorizedNumbers(const vector<wstring>& new
             // If the value is different, update the value
             if (MemorizedNumbers->GetAt(i)->Value != StringReference(newStringValue.c_str()))
             {
-                MemorizedNumbers->GetAt(i)->Value = Utils::LRO + ref new String(newStringValue.c_str()) + Utils::PDF;
+                MemorizedNumbers->GetAt(i)->Value = ref new String(newStringValue.c_str());
             }
         }
     }
@@ -1576,10 +1567,10 @@ void StandardCalculatorViewModel::UpdateProgrammerPanelDisplay()
     localizer.LocalizeDisplayValue(&octalDisplayString);
     localizer.LocalizeDisplayValue(&binaryDisplayString);
 
-    HexDisplayValue = Utils::LRO + ref new Platform::String(hexDisplayString.c_str()) + Utils::PDF;
-    DecimalDisplayValue = Utils::LRO + ref new Platform::String(decimalDisplayString.c_str()) + Utils::PDF;
-    OctalDisplayValue = Utils::LRO + ref new Platform::String(octalDisplayString.c_str()) + Utils::PDF;
-    BinaryDisplayValue = Utils::LRO + ref new Platform::String(binaryDisplayString.c_str()) + Utils::PDF;
+    HexDisplayValue = ref new Platform::String(hexDisplayString.c_str());
+    DecimalDisplayValue = ref new Platform::String(decimalDisplayString.c_str());
+    OctalDisplayValue = ref new Platform::String(octalDisplayString.c_str());
+    BinaryDisplayValue = ref new Platform::String(binaryDisplayString.c_str());
     HexDisplayValue_AutomationName = GetLocalizedStringFormat(m_localizedHexaDecimalAutomationFormat, GetNarratorStringReadRawNumbers(HexDisplayValue));
     DecDisplayValue_AutomationName = GetLocalizedStringFormat(m_localizedDecimalAutomationFormat, DecimalDisplayValue);
     OctDisplayValue_AutomationName = GetLocalizedStringFormat(m_localizedOctalAutomationFormat, GetNarratorStringReadRawNumbers(OctalDisplayValue));

diff --git a/src/CalcViewModel/StandardCalculatorViewModel.h b/src/CalcViewModel/StandardCalculatorViewModel.h
@@ -341,9 +341,9 @@ namespace CalculatorApp
             Platform::String ^ m_selectedExpressionLastData;
             Common::DisplayExpressionToken ^ m_selectedExpressionToken;
 
-            Platform::String ^ LocalizeDisplayValue(_In_ std::wstring const& displayValue, _In_ bool isError);
+            Platform::String ^ LocalizeDisplayValue(_In_ std::wstring const& displayValue);
             Platform::String
-                ^ CalculateNarratorDisplayValue(_In_ std::wstring const& displayValue, _In_ Platform::String ^ localizedDisplayValue, _In_ bool isError);
+                ^ CalculateNarratorDisplayValue(_In_ std::wstring const& displayValue, _In_ Platform::String ^ localizedDisplayValue);
             CalculatorApp::Common::Automation::NarratorAnnouncement ^ GetDisplayUpdatedNarratorAnnouncement();
             Platform::String ^ GetCalculatorExpressionAutomationName();
             Platform::String ^ GetNarratorStringReadRawNumbers(_In_ Platform::String ^ localizedDisplayValue);

diff --git a/src/CalcViewModel/UnitConverterViewModel.cpp b/src/CalcViewModel/UnitConverterViewModel.cpp
@@ -381,7 +381,6 @@ String ^ UnitConverterViewModel::ConvertToLocalizedString(const std::wstring& st
         }
         result = L"-" + result;
     }
-    result = Utils::LRE + result + Utils::PDF;
     return result;
 }
 

diff --git a/src/Calculator/App.xaml b/src/Calculator/App.xaml
@@ -577,7 +577,9 @@
                 <Setter Property="Template">
                     <Setter.Value>
                         <ControlTemplate TargetType="Controls:CalculationResult">
-                            <Grid x:Name="Border" Background="{TemplateBinding Background}">
+                            <Grid x:Name="Border"
+                                  Background="{TemplateBinding Background}"
+                                  FlowDirection="LeftToRight">
                                 <Grid.ColumnDefinitions>
                                     <ColumnDefinition Width="12"/>
                                     <ColumnDefinition/>

diff --git a/src/Calculator/Controls/CalculationResult.cpp b/src/Calculator/Controls/CalculationResult.cpp
@@ -215,7 +215,7 @@ void CalculationResult::UpdateTextState()
 
     auto containerSize = m_textContainer->ActualWidth;
     String ^ oldText = m_textBlock->Text;
-    String ^ newText = Utils::LRO + DisplayValue + Utils::PDF;
+    String ^ newText = DisplayValue;
 
     // Initiate the scaling operation
     // UpdateLayout will keep calling us until we make it through the below 2 if-statements

diff --git a/src/Calculator/Views/Calculator.xaml b/src/Calculator/Views/Calculator.xaml
@@ -37,7 +37,9 @@
             <Setter Property="Template">
                 <Setter.Value>
                     <ControlTemplate TargetType="controls:OverflowTextBlock">
-                        <Grid x:Name="TokenContainer" Background="{TemplateBinding Background}">
+                        <Grid x:Name="TokenContainer"
+                              Background="{TemplateBinding Background}"
+                              FlowDirection="LeftToRight">
                             <Grid.ColumnDefinitions>
                                 <ColumnDefinition Width="12"/>
                                 <ColumnDefinition/>
@@ -96,7 +98,9 @@
             <Setter Property="Template">
                 <Setter.Value>
                     <ControlTemplate TargetType="controls:OverflowTextBlock">
-                        <Grid x:Name="TokenContainer" Background="{TemplateBinding Background}">
+                        <Grid x:Name="TokenContainer"
+                              Background="{TemplateBinding Background}"
+                              FlowDirection="LeftToRight">
                             <Grid.Resources>
                                 <ResourceDictionary>
                                     <ResourceDictionary.ThemeDictionaries>

diff --git a/src/Calculator/Views/CalculatorProgrammerOperators.xaml b/src/Calculator/Views/CalculatorProgrammerOperators.xaml
@@ -163,7 +163,8 @@
     <Grid x:Name="ProgrammerOperators"
           x:Uid="RadixGroup"
           MaxHeight="244"
-          AutomationProperties.HeadingLevel="Level1">
+          AutomationProperties.HeadingLevel="Level1"
+          FlowDirection="LeftToRight">
         <Grid.RowDefinitions>
             <RowDefinition Height="1*" MinHeight="0"/>
             <RowDefinition Height="1*" MinHeight="0"/>

diff --git a/src/Calculator/Views/HistoryList.xaml b/src/Calculator/Views/HistoryList.xaml
@@ -2,10 +2,8 @@
              xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
              xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
              xmlns:automation="using:CalculatorApp.Common.Automation"
-             xmlns:controls="using:CalculatorApp.Controls"
              xmlns:converters="using:CalculatorApp.Converters"
              xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
-             xmlns:local="using:CalculatorApp"
              xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
              xmlns:model="using:CalculatorApp.ViewModel"
              xmlns:muxc="using:Microsoft.UI.Xaml.Controls"

diff --git a/src/Calculator/Views/MemoryListItem.xaml b/src/Calculator/Views/MemoryListItem.xaml
@@ -1,13 +1,8 @@
 <UserControl x:Class="CalculatorApp.MemoryListItem"
              xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
              xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
-             xmlns:common="using:CalculatorApp.Common"
-             xmlns:controls="using:CalculatorApp.Controls"
-             xmlns:converters="using:CalculatorApp.Converters"
              xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
-             xmlns:local="using:CalculatorApp"
              xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
-             xmlns:model="using:CalculatorApp.ViewModel"
              xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
              x:Name="MemoryListItem"
              FlowDirection="LeftToRight"

diff --git a/src/CalculatorUnitTests/CalculatorUnitTests.vcxproj b/src/CalculatorUnitTests/CalculatorUnitTests.vcxproj
@@ -237,7 +237,6 @@
     <ClCompile Include="CopyPasteManagerTest.cpp" />
     <ClCompile Include="CurrencyConverterUnitTests.cpp" />
     <ClCompile Include="DateCalculatorUnitTests.cpp" />
-    <ClCompile Include="Helper.cpp" />
     <ClCompile Include="HistoryTests.cpp" />
     <ClCompile Include="LocalizationServiceUnitTests.cpp" />
     <ClCompile Include="LocalizationSettingsUnitTests.cpp" />

diff --git a/src/CalculatorUnitTests/CalculatorUnitTests.vcxproj.filters b/src/CalculatorUnitTests/CalculatorUnitTests.vcxproj.filters
@@ -30,7 +30,6 @@
     <ClCompile Include="LocalizationServiceUnitTests.cpp" />
     <ClCompile Include="RationalTest.cpp" />
     <ClCompile Include="LocalizationSettingsUnitTests.cpp" />
-    <ClCompile Include="Helper.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="DateUtils.h" />

diff --git a/src/CalculatorUnitTests/Helper.cpp b/src/CalculatorUnitTests/Helper.cpp
diff --git a/src/CalculatorUnitTests/Helpers.h b/src/CalculatorUnitTests/Helpers.h
@@ -22,9 +22,6 @@ namespace CalculatorUnitTests
 
     namespace UtfUtils
     {
-        constexpr wchar_t LRE = 0x202a; // Left-to-Right Embedding
-        constexpr wchar_t PDF = 0x202c; // Pop Directional Formatting
-        constexpr wchar_t LRO = 0x202d; // Left-to-Right Override
         constexpr wchar_t MUL = 0x00d7; // Multiplication Symbol
     }
 }
@@ -108,5 +105,3 @@ void VERIFY_VECTORS_ARE_EQUAL(Windows::Foundation::Collections::IVector<T> ^ vec
         VERIFY_ARE_EQUAL(vecA->GetAt(i), vecB->GetAt(i), __VA_ARGS__);
     }
 };
-
-Platform::String ^ GetStringValue(Platform::String ^ input);
diff --git a/src/CalculatorUnitTests/HistoryTests.cpp b/src/CalculatorUnitTests/HistoryTests.cpp
@@ -80,7 +80,7 @@ namespace CalculatorFunctionalTests
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::CommandEQU));
             int sizeAfterItemAdd = m_historyViewModel->ItemSize;
             auto historyItem = static_cast<HistoryItemViewModel ^>(m_historyViewModel->Items->GetAt(0));
-            String ^ expression = UtfUtils::LRO + L"1   +   8 =" + UtfUtils::PDF;
+            String ^ expression = L"1   +   8 =";
             VERIFY_ARE_EQUAL(initialSize + 1, sizeAfterItemAdd);
             VERIFY_ARE_EQUAL(historyItem->Expression, expression);
             VERIFY_ARE_EQUAL(historyItem->Result, L"9");
@@ -102,7 +102,7 @@ namespace CalculatorFunctionalTests
                 m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::CommandEQU));
             }
             VERIFY_ARE_EQUAL((size_t)m_historyViewModel->ItemSize, m_historyViewModel->GetMaxItemSize());
-            String ^ expression = UtfUtils::LRO + L"1   +   1 =" + UtfUtils::PDF;
+            String ^ expression = L"1   +   1 =";
             int output = 2;
             String ^ result = output.ToString();
             auto historyItem = static_cast<HistoryItemViewModel ^>(m_historyViewModel->Items->GetAt(m_historyViewModel->ItemSize - 1));
@@ -112,7 +112,7 @@ namespace CalculatorFunctionalTests
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::CommandADD));
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::Command5));
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::CommandEQU));
-            expression = UtfUtils::LRO + L"1   +   2 =" + UtfUtils::PDF;
+            expression = L"1   +   2 =";
             output = 3;
             result = output.ToString();
             historyItem = static_cast<HistoryItemViewModel ^>(m_historyViewModel->Items->GetAt(m_historyViewModel->ItemSize - 1));
@@ -154,7 +154,6 @@ namespace CalculatorFunctionalTests
             for (int i = 0; i < scientificItems; i++)
             {
                 wstring expr = L"1   +   " + wstring(i.ToString()->Data()) + L" =";
-                expr = UtfUtils::LRO + expr + UtfUtils::PDF;
                 int output = 1 + i;
                 String ^ result = output.ToString();
                 auto historyItem = static_cast<HistoryItemViewModel ^>(m_historyViewModel->Items->GetAt(m_historyViewModel->ItemSize - 1 - i));
@@ -168,7 +167,6 @@ namespace CalculatorFunctionalTests
             for (int i = 0; i < standardItems; i++)
             {
                 wstring expr = L"1   +   " + wstring(i.ToString()->Data()) + L" =";
-                expr = UtfUtils::LRO + expr + UtfUtils::PDF;
                 int output = 1 + i;
                 String ^ result = output.ToString();
                 auto historyItem = static_cast<HistoryItemViewModel ^>(m_historyViewModel->Items->GetAt(m_historyViewModel->ItemSize - 1 - i));
@@ -238,8 +236,6 @@ namespace CalculatorFunctionalTests
             m_historyViewModel->ReloadHistory(ViewMode::Scientific);
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::ModeScientific));
             wstring expr = L"1   +   8 =";
-            // add double quotes around the expression
-            expr = UtfUtils::LRO + expr + UtfUtils::PDF;
             String ^ result = StringReference(L"9");
             int itemsAfterSaveAndReload = m_historyViewModel->ItemSize;
 
@@ -275,7 +271,6 @@ namespace CalculatorFunctionalTests
                     String ^ expression = m_uiResourceLoader->GetString(commandResource.ToString());
                     expression += L"( 1 ) =";
                     wstring expr = wstring(expression->Data());
-                    expr = UtfUtils::LRO + expr + UtfUtils::PDF;
                     VERIFY_ARE_EQUAL(historyItem->Expression, StringReference(expr.c_str()));
                     commandResource++;
                     itemIndex++;
@@ -309,7 +304,6 @@ namespace CalculatorFunctionalTests
             expression += m_uiResourceLoader->GetString(L"79");
             expression += L"( 1 ) =";
             wstring expr = wstring(expression->Data());
-            expr = UtfUtils::LRO + expr + UtfUtils::PDF;
             VERIFY_ARE_EQUAL(historyItem->Expression, StringReference(expr.c_str()));
 
             Cleanup();
@@ -406,13 +400,13 @@ namespace CalculatorFunctionalTests
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::Command7));
             m_standardViewModel->SendCommandToCalcManager(static_cast<int>(Command::CommandEQU));
             String ^ expression = L"HexaDecimal" + L" 8";
-            String ^ result = L"HexaDecimal " + GetStringValue(m_standardViewModel->HexDisplayValue);
+            String ^ result = L"HexaDecimal " + m_standardViewModel->HexDisplayValue;
             VERIFY_ARE_EQUAL(expression, result);
             expression = StringReference(L"Octal 10");
-            result = L"Octal " + GetStringValue(m_standardViewModel->OctalDisplayValue);
+            result = L"Octal " + m_standardViewModel->OctalDisplayValue;
             VERIFY_ARE_EQUAL(expression, result);
             expression = StringReference(L"Binary 1000");
-            result = L"Binary " + GetStringValue(m_standardViewModel->BinaryDisplayValue);
+            result = L"Binary " + m_standardViewModel->BinaryDisplayValue;
             VERIFY_ARE_EQUAL(expression, result);
             Cleanup();
         }
-Original file line number
+Diff line change
@@ Expand Up @@
             }
             result = L"-" + result;
         }
-        result = Utils::LRE + result + Utils::PDF;
         return result;
     }
@@ Expand Down @@