Skip to content

Razor Components (Blazor 0.8) - "double/float/decimal" error parse #7655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost opened this issue Feb 16, 2019 · 10 comments
Closed

Razor Components (Blazor 0.8) - "double/float/decimal" error parse #7655

ghost opened this issue Feb 16, 2019 · 10 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. ✔️ Resolution: Duplicate Resolved as a duplicate of another issue

Comments

@ghost
Copy link

ghost commented Feb 16, 2019

Describe the bug

If i use type="number" parameter I cannot use double / float / decimal in bind.

To Reproduce

@page "/testpage"

@functions {
    protected double sum { get; set; }
}

<input type="number" bind="@sum" />

When I enter 4.1 or 4,1 into input, I get an error message.

Uncaught (in promise) Error: System.FormatException: Input string was not in a correct format.
   at Microsoft.JSInterop.DotNetDispatcher.InvokeSynchronously(String assemblyName, String methodIdentifier, Object targetInstance, String argsJson)
   at Microsoft.JSInterop.DotNetDispatcher.BeginInvoke(String callId, String assemblyName, String methodIdentifier, Int64 dotNetObjectId, String argsJson)
    at endInvokeDotNetFromJS (components.server.js:23)
    at components.server.js:23
    at new Promise (<anonymous>)
    at e.beginInvokeJSFromDotNet (components.server.js:23)
    at components.server.js:16
    at Array.forEach (<anonymous>)
    at e.invokeClientMethod (components.server.js:16)
    at e.processIncomingData (components.server.js:16)
    at e.connection.onreceive (components.server.js:16)
    at WebSocket.i.onmessage (components.server.js:16)
components.server.js:16 Error: Connection disconnected with error 'Error: Websocket closed with status code: 1006 ()'.

Additional context

https://github.com/aspnet/AspNetCore/blob/master/src/Components/Components/src/BindMethods.cs#L155

public static Action<UIEventArgs> SetValueHandler(Action<double> setter, double existingValue)
{
   return _ =>
      {
         var val = (string)((UIChangeEventArgs)_).Value;  // res: 4.1  <= The value is always with a dot.
         var sum = double.Parse(val);                     // System.FormatException
         setter(sum);                                     // Not called
      };
}
@Andrzej-W
Copy link

Probably you have a system with decimal separator set to comma. Parse without culture uses current culture. They should use InvariantCulture. It looks that similar bug was introduced in new forms components:
#7614 (review)

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 17, 2019
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Mar 25, 2019
@mkArtakMSFT
Copy link
Contributor

@javiercn can you please try to reproduce this? Thanks!

@javiercn
Copy link
Member

After doing some investigations here are my conclusions.

  • In this case, the issue is the server parsing a value not using the invariant culture, so when the server is not in a culture compatible with the invariant (which normally is very much english) things can go wrong.
  • The invariant culture might not be the right choice all the time. It just so happens that when the input is of type number, the browser will normalize the value for us, but when the input does not have a type, that conversion doesn't happen.
  • I believe this means that potentially we need to somehow flow the culture from the browser to the server and potentially let the user make a choice on a per-binding basis.
  • Looking at what modelbinding does, it picks the culture from the currentculture, so if there is a middleware that sets that, then that culture gets used.
  • We don't have a similar mechanism in Blazor apps, so maybe this is something we need to add to the component context and enable setting somehow.

I'm by no means an internationalization expert. @rynowak do you have spicy thoughts on this topic?

As a plan of record, we should start by making things parse data using the invariant culture, and when we can see other options.

@javiercn javiercn added bug This issue describes a behavior which is not expected - a bug. cost: S and removed investigate labels Apr 12, 2019
@javiercn
Copy link
Member

Not sure how doable is to write an E2E test for this.

@javiercn
Copy link
Member

@cores-system What's the locale in your machine?

@ghost
Copy link
Author

ghost commented Apr 12, 2019

@javiercn, ru_RU

@javiercn
Copy link
Member

@cores-system Thanks.

Here is the confirmation:

double.Parse("1234.5", System.Globalization.CultureInfo.GetCultureInfo("ru-RU"))
Input string was not in a correct format.
  + System.Number.ParseDouble(string, System.Globalization.NumberStyles, System.Globalization.NumberFormatInfo)
  + double.Parse(string, System.IFormatProvider)
> double.Parse("1234,5", System.Globalization.CultureInfo.GetCultureInfo("ru-RU"))
1234.5

@javiercn
Copy link
Member

Looking forward, he main thing that can cause issues here is the fact that there might be normalized values and non normalized values at play depending on what you use type="number" or type="text" for example.

There might even be cases where choosing a different culture doesn't error out, but produces the wrong value.

It might be ok to say here that if you choose to bind to double it's invariant culture every single time, and that if you have type="text" you need to do the parsing yourself to a number or accept invariant culture as the culture.

That said, we should provide a way to flow the browser culture to the application. navigator.language seems to be the JS way of achieving this.

@danroth27 danroth27 added this to the 3.0.0-preview5 milestone Apr 12, 2019
@Andrzej-W
Copy link

I think that we should always use invariant culture in double/float/decimal binding. It should be documented that this will work correctly only with input type="number". Here is a small example to demonstrate comma/dot decimal separators in action.

<!DOCTYPE html>
<html>
    <body>
    <div>
        pl-pl <input lang="pl-pl" id="input-pl" type="number" step="0.01">
        <br>
        en-us <input lang="en-us" id="input-us" type="number" step="0.01">
        <br>
        <button onclick="setValues()">Set Values</button>
        <button onclick="getValues()">Get Values</button>
    </div>
    <div>
        pl-pl:<span id="value-pl"></span>
        <br>
        en-us:<span id="value-us"></span>
    </div>
    <script>
    function getValues(){
        document.getElementById("value-pl").innerHTML = document.getElementById("input-pl").value;
        document.getElementById("value-us").innerHTML = document.getElementById("input-us").value;
        }
    function setValues(){
        document.getElementById("input-pl").value = 1234.12;
        document.getElementById("input-us").value = 1234.12;
        }
    </script>
    </body>
</html>

In Firefox it works as expected: first input field displays numbers with coma decimal separator and second input field displays dot. Polish Chrome displays comma in both input fields which is in my opinion wrong. Fortunately value property always has dot.

I agree with @javiercn that if I want to use input type="text" I should bind to string property/variable and parse data myself.

@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@mkArtakMSFT
Copy link
Contributor

Closing this issue as this work will be covered as part of the referenced Globalization support issue.

@mkArtakMSFT mkArtakMSFT added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label May 28, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. ✔️ Resolution: Duplicate Resolved as a duplicate of another issue
Projects
None yet
Development

No branches or pull requests

5 participants