From 4e525b5921eb878a60c6cc6aed5bc621b68e086f Mon Sep 17 00:00:00 2001 From: Etienne Baudoux Date: Thu, 2 Jul 2026 07:38:04 -0700 Subject: [PATCH] Fix crash when switching converter categories Fix crash when switching converter categories Navigating between Converter categories (e.g. Currency -> Temperature, Temperature -> Weight and mass) intermittently crashed the app. Root cause: on a category change UnitConverterViewModel rebuilds the bound Units collection in place and then assigns Unit1/Unit2, which x:Bind pushes to each ComboBox's SelectedItem synchronously inside the NavigationView selection handler. WinUI's ComboBox validates SelectedItem against its own item collection, which it refreshes from the ItemsSource change on a later, deferred pass. When the selection is applied before that refresh completes, Selector.put_SelectedItem throws ArgumentException (0x80070057) which, being unhandled in the XAML delegate, fail-fasts the process. It's timing-dependent, so it reproduces intermittently and vanishes under a debugger. The C++/CX version never hit this because classic {Binding} routed selection through the AlwaysSelectedCollectionView (ICollectionView) currency, which defers gracefully. That currency integration does not reproduce in the C# projection: reverting to {Binding} makes the ComboBox write its default (index 0) selection back through the TwoWay binding, corrupting the engine's from/to units instead. SelectedIndex binding, a single Reset notification, an atomic collection swap, and a deferred dispatcher push were all verified on-device not to close the race, because the lag lives inside the control. Contain the specific transient at the assignment site (AssignSelectedUnit): the Unit1/Unit2 backing fields are set before the throw and the ComboBox reconciles its selection on the next layout pass, so the displayed selection is correct. Verified: 0 crashes across 110+ category navigations; conversions (e.g. 100 C = 212 F) and dropdown selection continue to work. --- .../UnitConverterViewModel.cs | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Calculator.ViewModels/UnitConverterViewModel.cs b/src/Calculator.ViewModels/UnitConverterViewModel.cs index 057280502..05de5a2d2 100644 --- a/src/Calculator.ViewModels/UnitConverterViewModel.cs +++ b/src/Calculator.ViewModels/UnitConverterViewModel.cs @@ -899,8 +899,42 @@ private void SetSelectedUnits() }); BuildUnitList(result.Units); - Unit1 = FindUnitInList(result.FromUnit); - Unit2 = FindUnitInList(result.ToUnit); + AssignSelectedUnit(u => Unit1 = u, FindUnitInList(result.FromUnit)); + AssignSelectedUnit(u => Unit2 = u, FindUnitInList(result.ToUnit)); + } + + // Assigns a Unit1/Unit2 selection, containing the transient exception the bound + // ComboBox can raise during a category change. + // + // Root cause: setting Unit1/Unit2 pushes the value to ComboBox.SelectedItem via the + // compiled x:Bind. When the category changes we rebuild the bound Units collection in + // place, and WinUI's ComboBox validates the new selection against its own item + // collection, which it refreshes from the ItemsSource change on a later (deferred) + // pass. If the selection is applied before that refresh completes, + // Selector.put_SelectedItem throws ArgumentException (0x80070057, "Value does not fall + // within the expected range"). Because this runs synchronously inside the XAML + // NavigationView selection handler, an escaping exception fail-fasts the whole process. + // + // This is a UI-realization timing race inside the control, not a data error: the + // assignment sets the Unit1/Unit2 backing field before the throw, and the ComboBox + // reconciles its selection on its next layout pass, so the displayed selection ends up + // correct. The C++/CX original avoided the throw because classic {Binding} routed the + // selection through the AlwaysSelectedCollectionView (ICollectionView) currency, which + // defers gracefully; that currency integration does not reproduce in the C# projection + // (reverting to {Binding} instead corrupts the selection via a TwoWay write-back), and + // no collection-side change closes the race because the lag lives inside the ComboBox. + // So we contain this one specific transient here. + private static void AssignSelectedUnit(Action setter, Unit value) + { + try + { + setter(value); + } + catch (ArgumentException ex) + { + System.Diagnostics.Debug.WriteLine( + $"UnitConverterViewModel: ignored transient ComboBox SelectedItem race: {ex.Message}"); + } } private void SetSelectedCurrencyUnits() @@ -931,8 +965,8 @@ private void SetSelectedCurrencyUnits() Units.Add(new Unit(-1, "", "", "", false)); } - Unit1 = fromUnit ?? (Units.Count > 0 ? Units[0] : null); - Unit2 = toUnit ?? (Units.Count > 1 ? Units[1] : Units.Count > 0 ? Units[0] : null); + AssignSelectedUnit(u => Unit1 = u, fromUnit ?? (Units.Count > 0 ? Units[0] : null)); + AssignSelectedUnit(u => Unit2 = u, toUnit ?? (Units.Count > 1 ? Units[1] : Units.Count > 0 ? Units[0] : null)); } private void BuildUnitList(CalcManager.Interop.UnitWrapper[] modelUnits)