ColorPicker: Fix app crash when clearing input fields (#3224)

* Fix crash issues. * Added API tests.
microsoft · Sep 2, 2020 · 6097584 · 6097584
1 parent 647cb4d
commit 6097584
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 6 deletions.
diff --git a/dev/ColorPicker/APITests/ColorPickerTests.cs b/dev/ColorPicker/APITests/ColorPickerTests.cs
@@ -253,6 +253,58 @@ public void ValidateFractionalWidthDoesNotCrash()
             SetAsRootAndWaitForColorSpectrumFill(colorSpectrum);
         }
 
+        [TestMethod]
+        public void VerifyClearingHexInputFieldDoesNotCrash()
+        {
+            ColorPicker colorPicker = null;
+
+            RunOnUIThread.Execute(() =>
+            {
+                colorPicker = new ColorPicker();
+                colorPicker.IsHexInputVisible = true;
+
+                Content = colorPicker;
+            });
+
+            IdleSynchronizer.Wait();
+
+            RunOnUIThread.Execute(() =>
+            {
+                var hexTextBox = VisualTreeUtils.FindVisualChildByName(colorPicker, "HexTextBox") as TextBox;
+
+                Verify.IsGreaterThan(hexTextBox.Text.Length, 0, "Hex TextBox should have not been empty.");
+
+                // Clearing the hex input field should not crash the app.
+                hexTextBox.Text = "";
+            });
+        }
+
+        [TestMethod]
+        public void VerifyClearingAlphaChannelInputFieldDoesNotCrash()
+        {
+            ColorPicker colorPicker = null;
+
+            RunOnUIThread.Execute(() =>
+            {
+                colorPicker = new ColorPicker();
+                colorPicker.IsAlphaEnabled = true;
+
+                Content = colorPicker;
+            });
+
+            IdleSynchronizer.Wait();
+
+            RunOnUIThread.Execute(() =>
+            {
+                var alphaChannelTextBox = VisualTreeUtils.FindVisualChildByName(colorPicker, "AlphaTextBox") as TextBox;
+
+                Verify.IsGreaterThan(alphaChannelTextBox.Text.Length, 0, "Alpha channel TextBox should have not been empty.");
+
+                // Clearing the alpha channel input field should not crash the app.
+                alphaChannelTextBox.Text = "";
+            });
+        }
+
         // XamlControlsXamlMetaDataProvider does not exist in the OS repo,
         // so we can't execute this test as authored there.
         [TestMethod]

diff --git a/dev/ColorPicker/ColorPicker.cpp b/dev/ColorPicker/ColorPicker.cpp
@@ -923,11 +923,10 @@ void ColorPicker::OnAlphaTextChanging(winrt::TextBox const& /*sender*/, winrt::T
         // where it was before.
         const int cursorPosition = alphaTextBox.SelectionStart() + alphaTextBox.SelectionLength();
 
-        const wchar_t* s = alphaTextBox.Text().begin() + alphaTextBox.Text().size() - 1;
-
-        if (*s != '%')
+        const auto text = alphaTextBox.Text();
+        if (text.empty() || text.begin()[text.size() - 1] != '%')
         {
-            alphaTextBox.Text(static_cast<wstring>(alphaTextBox.Text()) + wstring(L"%"));
+            alphaTextBox.Text(static_cast<wstring>(text) + wstring(L"%"));
             alphaTextBox.SelectionStart(cursorPosition);
         }
 
@@ -961,9 +960,10 @@ void ColorPicker::OnHexTextChanging(winrt::TextBox const& /*sender*/, winrt::Tex
 
     // If the user hasn't entered a #, we'll do that for them, keeping the cursor
     // where it was before.
-    if (hexTextBox.Text().begin()[0] != '#')
+    const auto text = hexTextBox.Text();
+    if (text.empty() || text.begin()[0] != '#')
     {
-        hexTextBox.Text(wstring(L"#") + static_cast<wstring>(hexTextBox.Text()));
+        hexTextBox.Text(wstring(L"#") + static_cast<wstring>(text));
         hexTextBox.SelectionStart(hexTextBox.Text().size());
     }