Windowscommunitytoolkit: DataGrid Column Proportional Widths with Decimal Separator is Dependent on CurrentCulture

[thom0707]

Describe the bug

DataGrid column proportional widths with the decimal separator are dependent on CurrentCulture and can cause runtime exceptions to occur if the CurrentCulture is changed to a value that uses a different decimal separator than what is provided in the XAML.

Steps to Reproduce

Steps to reproduce the behavior:

1. Add a DataGrid with at least one column with a proportional width including a decimal (e.g. ".3*").
2. Change the CurrentCulture to a culture that uses a non-dot decimal separator (e.g. "fr-FR").
3. Run the application.
4. See runtime exception: "System.FormatException: 'Input string was not in a correct format.'"

Expected behavior

No runtime exception should be raised.

Additional thoughts

• The issue can be worked around by using whole numbers as proportional widths. This is my personal preference.
• Although the issue can be worked around, it appears that fractional values are legal. I cannot find documentation to prove or disprove this at this time.
• If the CurrentCulture is "fr-FR" (for example), ",3*" appears to be a legal width in XAML (further supporting the results found).

This issue can theoretically occur in at least the following scenarios:

1. CurrentCulture is changed at runtime with a decimal separator in conflict with the decimal separator in the XAML column width.
2. Source code is built on a machine with a CurrentCulture setting in conflict with the decimal separator in the XAML column width. This makes the source code less portable.

Environment

NuGet Package(s): 
Microsoft.Toolkit.Uwp.UI.Controls.DataGrid

Package Version(s): 
v6.0.0

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] Insider Build (build number: )
- [x] November 2019 Update (18383)

App min and target version:
- [x] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] Insider Build (xxxxx)
(Appears to be a problem with other versions, but the app this was initially found and reproduced in targeted 16299 [min and target]).

Device form factor:
- [x] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [ ] 2017 (version: )
- [x] 2019 (version: 16.3.8) 
- [ ] 2019 Preview (version: )

Additional context

Submitted a sample app reproducing the problem here.

The following generated code is where the exception surfaces (XamlTypeInfo.g.cs):
case 15: // Microsoft.Toolkit.Uwp.UI.Controls.DataGridLength userType = new global::DataGrid_Culture_Issue.DataGrid_Culture_Issue_XamlTypeInfo.XamlUserType(this, typeName, type, GetXamlTypeByName("System.ValueType")); userType.CreateFromStringMethod = x => (global::System.Object)global::Microsoft.Toolkit.Uwp.UI.Controls.DataGridLength.ConvertFromString(x); userType.SetIsReturnTypeStub(); xamlType = userType; break;

In the source code, it's easy to see where the problem is occurring. return ConvertFrom(null, value); is called, with the null parameter being the CultureInfo value passed. This eventually gets to the following line:

starWeight = Convert.ToDouble(stringValueWithoutSuffix, culture ?? CultureInfo.CurrentCulture);

This is where the CurrentCulture is used to convert the string value, potentially raising a System.FormatException.

[anawishnoff]

Thanks so much for all of the details you've provided here, @Tominator-T1000! It seems like you've really got to the bottom of this bug and it should be a relatively simple fix...

@RBrid - what are your thoughts on fixing this bug?

[anawishnoff]

@Tominator-T1000, sorry for the delay on this. It looks like you were correct in your assumption of where the problem is stemming from. Our engineers working on DataGrid have been super busy so we're hoping to get some community involvement to help out on this bug.

If you feel like you'd like to look into this bug further, you're totally welcome to! It looks like you already have a head-start from doing the initial investigation.

[thom0707]

@anawishnoff I have been hoping to get more involved in the community so I may try to pick this up. I did try a while back but had issues getting the codebase to build. I may give it another shot when I have some free time.

[thom0707]

@anawishnoff I'd like to take a stab at fixing this bug, but I could use some direction. I can't find documentation that a decimal separator other than "." is legal XAML. This is important because I feel the solution should adhere to XAML standards and I want to fix this correctly. If "0,5" is entered, even in a culture that uses a "," as the decimal separator, should this be allowed? Based on my research so far, code portability concerns, and some testing on a vanilla UWP project, it seems like the answer is "no". However, some feedback on that would be helpful before I get started. If you or someone on your team could provide guidance, I'd appreciate it.

[anawishnoff]

Hey @thom0707 thanks for taking this on! I spoke with one of our engineers about your question and this was his response:

The Xaml markup language is en-us, so the only supported syntax for numbers is “.” as a decimal place.

I think that WPF may have supported the "0,5" syntax but WinUI does not. Hope that helps!