Fix uncontrolled input decimal point "chopping" on number inputs, and validation warnings on email inputs

[nhunzaker]

This fix is stuck inside of #7359 until I can sort out the outstanding concerns around controlled inputs. Hopefully this is easier to merge and people can start using uncontrolled number inputs again!

Basically avoid assigning defaultValue unless you have to. This prevents Chrome's number inputs from dropping decimal places or clearing the input when typing 3e. As documented here: #7253 (comment)

[quantizor]

Honestly, I'm a bit confused why React changes defaultValue and defaultChecked at all after initial mount. Seems contradictory.

[nhunzaker]

@yaycmyk I can confirm later today, but I believe it is to support an input changing from controlled to uncontrolled. I think this technically should raise a console warning from React, but there's definitely a test for it.

I can dig in and explain more later today.

[nhunzaker]

@yaycmyk Yeah, I believe it's needed to allow this test to pass:

https://github.com/facebook/react/blob/master/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js#L70-L81

[nhunzaker]

Beyond that, unsure to to ping, but I'd love to get this through if possible. Anything I can do to help move this along?

[quantizor]

Yeah I was more commenting that the behavior and accompanying test may not actually make sense. Would love to get a collaborator's opinion like @zpao

[aweary]

I've reviewed this (when it was part of #7359) and it looks decent to me, but I would like someone from the core team to give the OK here.

[nhunzaker]

@aweary Awesome. I feel like I've taken you down a rocky road with all of this, thank you for your help!

[nhunzaker]

Upstreamed with master.

[martin-svk]

Hello, is there a timeline for this PR to be merged and released?

[nhunzaker]

Any testing materials I can put together to help verify this fix? I'd really like this to ship too.

[gaearon]

Can you please provide a table of the browsers you tested, test cases, and the behavior before and after this change?

[nhunzaker]

To reproduce the issue, visit here:
http://natehunzaker.com/react-issue-reproducer/cases/input/

When, in Chrome, backspacing to 3 in the uncontrolled number input, you'll see the decimal place gets "lost":

This fix is hosted here:
http://natehunzaker.com/react-issue-reproducer/edge/cases/input

And here's the list of browsers (though Chrome is the only place this matters):

Browser	Before	After
Chrome 53.0.2785.143	Decimal place is incorrectly "lost"	No removal
Safari 10	No removal	No removal
Safari iOS 9	No removal	No removal
Firefox Dev Edition 51.0a2	No removal	No removal
Firefox 49	No removal	No removal
IE Edge (14)	No removal	No removal
IE 11	No removal	No removal
IE 10	No removal	No removal

This table is a bit excessive, but at least I got an excuse to use the markdown table generator for Emacs 😀 🔌.

Just to reiterate, this appears to be specific to Chrome. It happens because Chrome updates the value of a number input whenever the value property, defaultValue property, or value attribute is changed. This PR fixes the issue by avoiding assignment unless the value has actually changed.

[aweary]

This fix is hosted here:
http://natehunzaker.com/react-issue-reproducer/edge/cases/input

@nhunzaker I'm still seeing the issue on this page?

[nhunzaker]

@aweary Sorry :(. That was confusing. @gaearon how does this sound:

// In Chrome, assigning defaultValue to certain input types triggers input validation.
// For number inputs, the display value loses trailing decimal points. For email inputs,
// Chrome raises "The specified value <x> is not a valid email address".
//
// Here we check to see if the defaultValue has actually changed, avoiding these problems
// when the user is inputting text
//
// https://github.com/facebook/react/issues/7253

Too verbose?

[aweary]

Ah, I misread 😄 I think this change is reasonable. It makes sense regardless not to set an attribute unless it's required.

[gaearon]

@nhunzaker Sounds great, there's never a verbose Chrome bug description.

[nhunzaker]

Done! Now if I can only figure out how to solve #7359 ....

[aweary]

@gaearon look good to you? I think this is safe to merge 👍

[gaearon]

Do it

[nhunzaker]

[gaearon]

@aweary Please don't forget to tag the 15-next milestone on merge.