Localised number format and wrong input type for decimal fields

[ygoe]

Describe the bug

With a model that contains a decimal property, this doesn't work with a locale that uses a decimal comma:

<input asp-for="Number">
<input asp-for="Number" type="number">

To Reproduce

Steps to reproduce the behavior:

1. Using this version of ASP.NET Core 2.1.5
2. Create the model class and the HTML razor view, then run it

Code:

class Model
{
    public decimal Number { get; set; } = 1.0m;
}

Expected behavior

This should be the rendered HTML:

<input type="number" name="Number" value="1.0">

This happens instead, for the two view variants from the top:

<input type="text" name="Number" value="1,0">
<input type="number" name="Number" value="1,0">

The first issue is that the decimal type isn't rendered with the type="number" attribute. If I fix this myself by adding the attribute, the field remains completely empty because there is a comma instead of a point in the value. This shouldn't be localised because then no browser or frontend library can handle the value anymore.

PS: I haven't even got to try what happens when the correct value "1.0" is sent back to the controller and the model binder should convert it to a decimal type. It probably fails for the same reason.

[User reference: Configuration/Endpoint/Edit/Factor]

[AndreasAmMueller]

Ran into the same bug and helped me out with a custom TagHelper that recognizes the For.ModelExplorer.ModelType as decimal or nullable decimal and then modifies the value attribute.

if (For.Model != null)
{
	decimal val = (decimal)For.Model;
	string value = val.ToString(CultureInfo.InvariantCulture);
	output.Attributes.SetAttribute(new TagHelperAttribute("value", value));
}

You also might want to set the "step" attrbute to "any".

For the .NET Core MVC Team:
I found some lines that are part of the bug, but I was not able to solve the wohle problem.

Lines found:

• The type should result in "number" not "text": https://git.1-hub.cnaspnet/AspNetCore/blob/1aa50faa290ecda304507981cd01ed92651d5e34/src/Mvc/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs#L51
• This line has the CultureInfo.CurrentCulture (for me, its de-DE!) as value. CultureInfo.InvariantCulture is needed to parse correctly: https://git.1-hub.cnaspnet/AspNetCore/blob/1aa50faa290ecda304507981cd01ed92651d5e34/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/DecimalModelBinder.cs#L80

[mkArtakMSFT]

Thanks for contacting us. We believe that the question you've raised have been answered. If you still feel a need to continue the discussion, feel free to reopen it and add your comments.

[ygoe]

@mkArtakMSFT Can you please explain why you believe the question has been answered / the bug has been fixed? I cannot see how this is the case. Please reopen until this has been fixed. In case you didn't know, there are other regions in the world than those that use a decimal point. And currently I can't use proper HTML with decimal numbers with ASP.NET Core MVC data binding.

[mkArtakMSFT]

Looks like I misread this. Reopening for further investigation.

[ygoe]

@mkArtakMSFT Did you want to reopen this issue?

[mkArtakMSFT]

type="number" fields use invariant culture and we should consider parsing as such in model binding.

[StefanOssendorf]

Any progress on this? I hit the same behavior with Blazor WebAssembly though I think it also applies to Blazor Server Side. The same applies to the min, max and step attribute of an input-tag of type number or range.

[Kurti123]

Experiencing the same issue on latest 3.1 stream of .NET Core. I have German culture, so we use "," as decimal separator. When I send a decimal value from an input field to the controller, the value "214,50" is sent to the controller. But when mapped to my ViewModel in the controller, the comma is lost and the value 21450 in mapped to my models attribute which is a decimal type...

However, with a piece of old legacy code within the same solution, the mapping works just fine although the setup is exactly the same.

EDIT: I just found out that the error only occurs when I use GET. When I send the data via POST, the binding works just fine.

[mkArtakMSFT]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[yonder83]

Same issue for me. My development environment (Visual Studio) is in English, my browser in French, and this makes decimal datatype unusable (native .Net JS validation is enabled), neither with a dot, nor a comma. Below the screens.

Dot is unusable (ex 1.5):

Coma is unusable (ex: 1,5):

The latter error is caused because you are using jquery-validation. It has a function that validates number, it has nothing to do with browser language. You can test it here https://jqueryvalidation.org/number-method/.

To get comma validated as a decimal separator, you can override the jquery-validation number validation as suggeted by @Nikkelmann #6566 (comment)

[remcoros]

According to HTML standard, the 'value' of a number input element should always be in the invariant culture. Browsers may choose to render it differently based on the users browser settings.

From what I remember this has been a source of pain since Webforms / MVC 1.

[muhlisatac]

I'm tired of searching for a permanent solution to this problem in every new project for years. Can you please solve this problem in 7.0-rc1 milestone?

[halter73]

Background and Motivation

@MackinnonBuck writes,

I’m working on a fix for MVC form data binding that changes HTML generation for form inputs. Since this is technically a breaking change, we’re going to introduce a new flag that allows customers to revert to the old behavior. The default value for this flag (false) enables the new behavior.

Proposed API

namespace Microsoft.AspNetCore.Mvc.ViewFeatures;

public class HtmlHelperOptions
{
+    public bool SuppressCultureInvariantFormValueFormatting { get; set; }
}

namespace Microsoft.AspNetCore.Mvc.Rendering;

public class ViewContext
{
+    public bool SuppressCultureInvariantFormValueFormatting { get; set; }
}

Setting this flag to true does two things:

1. Always uses CultureInfo.CurrentCulture to format the value attribute for HTML inputs.
2. Prevents the new hidden <input/> elements from being generated (these contain metadata about which form values use invariant formatting).

Usage Examples

public void ConfigureServices(IServiceCollection services)
{
    services.AddMvc()
        .AddViewOptions(options =>
        {
            options.HtmlHelperOptions.SuppressCultureInvariantFormValueFormatting = true;
        });
}

Alternative Designs

Alternate names:

• AlwaysUseCurrentCultureInFormValueFormatting
• TreatAllFormInputsAsCultureSpecific
• Other suggestions?

Risks

N/A

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

@captainsafia writes,

Have you considered the name CultureInvariantFormValueFormattingEnabled​? it seems to match the naming style used for the other boolean-based option (ClientValidationEnabled) in the HtmlHelperOptions​ and ViewContext​ classes.

[halter73]

How likely do we think it is that people will want to change this? Given the description of the PR it seems like we're just correcting behavior but we're adding an escape hatch out of an abundance of caution. Can we make this an AppContext switch instead of a public API?

[MackinnonBuck]

Have you considered the name CultureInvariantFormValueFormattingEnabled​? it seems to match the naming style used for the other boolean-based option (ClientValidationEnabled) in the HtmlHelperOptions​ and ViewContext​ classes.

I like this name.

Can we make this an AppContext switch instead of a public API?

It looks like we've already used HtmlHelperOptions to configure changes to HTML generation (for example, Html5DateRenderingMode and CheckBoxHiddenInputRenderMode). Do you think there's any merit to adding a new property in HtmlHelperOptions instead of using an AppContext switch in order to keep things in the same place?

[MackinnonBuck]

Adding on to my previous comment, Html5DateRenderingMode and CheckBoxHiddenInputRenderMode are both enum values. If we decide to add a new API at all, should we make it an enum in case we decide to make further changes in the future?

[halter73]

Do you think there's any merit to adding a new property in HtmlHelperOptions instead of using an AppContext switch in order to keep things in the same place?

I do think there's merit to adding options to HtmlHelperOptions given similar options already exist.

Adding on to my previous comment, Html5DateRenderingMode and CheckBoxHiddenInputRenderMode are both enum values. If we decide to add a new API at all, should we make it an enum in case we decide to make further changes in the future?

I like that idea. What would you name the enum? FormValueFormattingMode? What about the values? InvariantCulture and CurrentCulture? Can you leave a comment with the new proposed API with the enum?

By the way, how does this new API interact with the existing HtmlHelperOptions does it override or get overridden by any of them?

Also, can you explain how the HtmlHelperOptions relates to the ViewContext and the DefaultHtmlGenerator? Why are Html5DateRendingMode and CheckBoxHiddenInputRenderMode on the ViewContext but IdAttributeDotReplacement is on the DefaultHtmlGenerator? Would the ViewContext API work just as well if it were on the DefaultHtmlGenerator?

[MackinnonBuck]

Proposed API

namespace Microsoft.AspNetCore.Mvc.Rendering;

+public enum FormInputRenderMode
+{
+    DetectCultureFromInputType = 0,
+    AlwaysUseCurrentCulture = 1,
+}

namespace Microsoft.AspNetCore.Mvc.ViewFeatures;

public class HtmlHelperOptions
{
+    public FormInputRenderMode FormInputRenderMode { get; set; }
}

Usage Examples

public void ConfigureServices(IServiceCollection services)
{
    services.AddMvc()
        .AddViewOptions(options =>
        {
            options.HtmlHelperOptions.FormInputRenderMode = FormInputRenderMode.AlwaysUseCurrentCulture;
        });
}

[MackinnonBuck]

how does this new API interact with the existing HtmlHelperOptions does it override or get overridden by any of them?

It does not interact with other properties - it's completely independent.

Also, can you explain how the HtmlHelperOptions relates to the ViewContext and the DefaultHtmlGenerator?

HtmlHelperOptions can be accessed via MvcViewOptions for classes that have it injected via DI. For classes that don't (e.g. tag helpers), the ViewContext has its own copies of the properties in HtmlHelperOptions that can be read (why it doesn't cache a HtmlHelperOptions and expose that as a public property, I'm not sure, but that's not something we can reasonably change at this point).

Why are Html5DateRendingMode and CheckBoxHiddenInputRenderMode on the ViewContext but IdAttributeDotReplacement is on the DefaultHtmlGenerator?

Presumably it's because properties on ViewContext needed to be used by tag helpers, but IdAttributeDotReplacement did not. But follwing that logic, we could remove this new property from ViewContext (an older version of my PR did reference it in InputTagHelper, but as things currently stand, we could safely remove it). In fact, I've gone ahead and done that in the API proposal in my previous comment, so thanks for bringing it up!

[halter73]

The above API makes sense to me. Do we prefer "render" mode over "rendering" mode? Html5DateRenderingMode and CheckBoxHiddenInputRenderMode are already inconsistent here, so I could go either way. If you prefer "render", I'm good with approving your last proposal as-is.

[MackinnonBuck]

I like that idea. What would you name the enum? FormValueFormattingMode?

I think FormInputRenderMode might be good because FormValueFormattingMode doesn't necessarily represent the fact that this option also controls whether hidden <input name="__Invariant" ... /> elements are generated.

What about the values? InvariantCulture and CurrentCulture?

I've chosen DetectCultureFromInputType instead of InvariantCulture because it still might use CurrentCulture for some input types.

And I think AlwaysUseCurrentCulture highlights that while the other option may use CurrentCulture, this one will definitely use it.

[MackinnonBuck]

Do we prefer "render" mode over "rendering" mode?

Hm, I really don't have a preference here. Maybe I like "render" a bit more just because the "ing" doesn't really add any important information. Totally willing to change my mind though!

[halter73]

If there are no objections, I can mark this api-approved Monday. I want to give a little more time for other people to provide input though.

[ygoe]

As I understood it, the new default will be the fixed InvariantCulture behaviour, so I have no interest in this API and don't care about its name. But keep in mind that this API will retain the current bug behaviour. It will generate invalid or at least unusable HTML output that cannot be understood by web browsers. There should be a good warning attached to it that its only purpose is to retain compatibility with old bugs. But it seems the other two HtmlHelperOptions already serve the same purpose.

[halter73]

API review notes:

• Do we need this API at all?
  • If this is just a bug fix, can we get away with using just an AppContext switch?
  • This has been the behavior since before ASP.NET core. We are afraid people rely on the old behavior, and this new API isn't exactly in your face.

API Approved!

[valeriob]

Hi,
I may be missing something, but it will be difficult to apply this approach in a big application where you will have to incrementally migrate actions from one model to another to leverage the modern browser capabilities, is there a way to have a per request configuration ?