React: Bug: Backspace in input type="number" behaves badly in Blink
This appears to have been introduced in a new Chrome version, but I can't find any reference to where.
Affected/Tested Browsers (OS X):
- Chrome 51.0.2704.106 (64-bit)
- Opera 39.0.2256.15
Unaffected Browsers:
- Safari 9.1
- Firefox 49
Backspacing in an input element with value or defaultValue set causes some very odd behavior. Once a decimal point is gone, it can't easily be re-added.
Example:
In this example, I simply backspaced twice. On the second backspace, when I expect 3. to be showing, the input instead reads 3 and the cursor has moved to the beginning. The next two jumps are my attempts to add another decimal point.
Fiddle: https://jsfiddle.net/kmqz6kw8/
Tested with React 15.2.
Notes: This only occurs when value or defaultValue is set. If neither is set, the input behaves properly. We are currently working around this issue by (unfortunately) setting the input value on componentDidMount via a ref.
STRML
All 29 comments
😢 Thanks for filing. I confirmed 15.2.1 is still broken too. Most likely more fallout from #6406.
zpao
on 13 Jul 2016
Number inputs are broken in React, due to https://github.com/facebook/react/issues/6556, which was filed before #6406 was merged. You're going to see the same problem with email inputs (as per https://github.com/facebook/react/issues/6368, which is basically the same issue, again filed before #6406 was merged).
It is entirely possible that #6406 exposes that bug for defaultValue, but the bug was always there in the React core and was always visible for controlled inputs.
This has been broken in React for as long as I can remember:
var NumberInput = React.createClass({
getInitialState: function() { return {value: 3.14}; },
render: function() {
return <input type="number" value={this.state.value} onChange={(e)=>{this.setState({value: e.target.value});}} />;
}
});
ReactDOM.render(
<NumberInput />,
document.getElementById('container')
);
A workaround is to set type=text and do manual value filtering to ensure that only numbers are entered into the textbox. This has the downside that you don't get the numeric keyboard on mobile, but we'll need to figure out a solution to https://github.com/facebook/react/issues/6556 to handle that properly.
jimfb
on 18 Jul 2016
@jimfb Understood, it's very frustrating that there is no way to get the raw value out of an <input type="number">. I think, though, that it deserves some investigation into why this bug _does not_ manifest in Firefox, as I believe it still follows the spec on that one.
STRML
on 18 Jul 2016
@STRML That's because Firefox doesn't fire an onChange if the input's value is invalid. It's an interesting way of sidestepping the issue in the common case, since the onChange behavior is presumably undefined when the input is in an illegal state.
jimfb
on 18 Jul 2016
This has been broken in React for as long as I can remember
Based on these two codepens, this bug was introduced in 15.2.0.
15.1.0: http://codepen.io/stephenjwatkins/pen/GqxGWJ [works]
15.2.0: http://codepen.io/stephenjwatkins/pen/yJpWQk [doesn't work]
stephenjwatkins
on 21 Jul 2016
Wow, did Chorme just release an update? I'm 98% positive that codepen would not have worked a week or two ago.
It looks like Chrome will now retain the "." when the user does a input.value=3
The fiddles in #6556 are now all behaving in a "nicer" way now. Looks like the ball is back in our court.
How much you want to bet that Chrome fixed this because of React?
jimfb
on 21 Jul 2016
Still broken in 52.0.2743.82 (64-bit), are you seeing this working with 15.2.0?
STRML
on 21 Jul 2016
What is still broken? I'm talking about the fiddles in #6556. Do those still look broken to you?
jimfb
on 21 Jul 2016
Yes - on Chrome 52
Edit: I see what you mean, the following sequence now behaves properly:
- Type
3-onInputcalls back with'3' - Type
.-onInputcalls back with'3', previously'' - Type
1,onInputcalls back with'3.1'
STRML
on 21 Jul 2016
Confirmed for me on:
Version 52.0.2743.82
Specifically, it looks like input.value is lopping off the decimal point:
To reproduce:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Chrome Backspace Issue</title>
</head>
<body>
<div id="container"></div>
<script src="../../build/react.js"></script>
<script src="../../build/react-dom.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/babel-core/5.8.24/browser.min.js"></script>
<script type="text/babel">
var NumberInput = React.createClass({
render: function() {
return <input type="number" defaultValue={ this.props.defaultValue } />;
}
});
ReactDOM.render(
<NumberInput defaultValue={3.14} />,
document.getElementById('container')
);
</script>
</body>
</html>
If you backspace to clear the 14 in 3.14, Chrome is reporting input.value to be 3, instead of 3..
Do I have this right? And should uncontrolled inputs even assign values to begin with?
nhunzaker
on 26 Jul 2016
Would it be crazy to consider not emitting changes for invalid input? In the case I've detailed above, the stepMismatch validation returns:
nhunzaker
on 26 Jul 2016
Ahhhh interesting. Maybe I was just in the dark, but assigning defaultValue causes side-effects on value. It seems to trigger validation on value.
Check out this Codepen:
http://codepen.io/nhunzaker/pen/zBaokp
This covers uncontrolled inputs, but controlled inputs follow different rules and seem to rely on value, which always reports 3 instead of 3. for me.
I've sent out a PR here:
nhunzaker
on 27 Jul 2016
@jimfb You mentioned:
That's because Firefox doesn't fire an onChange if the input's value is invalid. It's an interesting way of sidestepping the issue in the common case, since the onChange behavior is presumably undefined when the input is in an illegal state.
This seems ideal to me: to trade off limiting change exposure for the sake of stable inputs. Is there any interest in moving towards this?
nhunzaker
on 27 Jul 2016
@nhunzaker Sorry, I'm on hackamonth, haven't really been following these threads super closely.
We can't mimic Firefox's behavior, because we can differentiate between the user clearing their text input and an invalid value, AFAIK.
However, I think Chrome fixed their inputs to work correctly, so this class of issues is now fixable. We would almost certainly be interested in a PR. I'll pass this off to @zpao @spicyj and @gaearon.
jimfb
on 27 Jul 2016
No worries, sounds fun!
If there's a quick reference point, could you reveal more about the Chrome fix? What was actually broken?
nhunzaker
on 27 Jul 2016
@nhunzaker I don't know any quick reference points other than https://github.com/facebook/react/issues/6556, but the basic problem was that we couldn't get the actual input value (as entered by the user) when the input was in an invalid state (eg. as the user is typing "3.14", there is an illegal intermediate state "3.") and setting the last known legal value would remove the decimal point. Firefox's fix was to not fire onInput for illegal states, Chrome's fix was to not remove the decimal if the value is approximately the same ("3" instead of "3.").
You'll need to do a little digging to figure out the exact behaviors of the various browsers and figure out what a good fix would look like. I don't know the answer off the top of my head, but it looks like things are now sufficiently fixed on the browser's end, so this should be fixable now in React.
FWIW, it may be worth tracking keystrokes and cursor events and paste events, and maintain a perfect shadow state. Since we don't have to deal with non-numeric-ish characters, we don't need to worry about foreign languages (which is where all the input edge cases come from). I _think_ this would allow us to simulate what the browser would do (allow us to know what value is actually in the text box) and sidestep all the weirdness of the type=number spec. Anyway, I'll let you guys figure that out.
jimfb
on 27 Jul 2016
Okay! So it looks like the issue here is that value is being assigned using the standard React method of updating attributes when the props get passed in here:
https://github.com/facebook/react/blob/master/src/renderers/dom/shared/ReactDOMComponent.js#L926
There is some logic in ReactDOMInput to prevent duplicate values from being assigned here:
Unfortunately the new value has already been assigned.
So my first quick hack was to add an exception for value assignment so that it never sets the value attribute if it hasn't changed:
https://github.com/facebook/react/compare/master...nhunzaker:nh-chrome-backspace
I don't really know why this works. I didn't think node.setAttribute('value', value) was meaningful. Still, it fixes all of the issues I've encountered.
nhunzaker
on 29 Jul 2016
Here's a quick GIF:
Now it looks like I was dumb and pushed this update to the uncontrolled input fix I sent out here:
https://github.com/facebook/react/pull/7359
I'll work on getting those tests to pass until we can figure out what to do.
nhunzaker
on 29 Jul 2016
Okay. I've polished up https://github.com/facebook/react/pull/7359. It fixes all issues with uncontrolled and controlled inputs that I have encountered.
I'll put in some extra time to manually test this tomorrow. I'm still struggling to add automated test coverage for these quirks.
nhunzaker
on 29 Jul 2016
Here are the cases that I'm covering, using the following code as a test case:
https://gist.github.com/nhunzaker/923efbaddca16ae2384b547555157f61
Do not drop off the decimal place when backspacing
This happens because React sees "3." and tries to compare it against "3". Since it's a string comparison, these are always different.
before:
after:
Do not clear on bad input
This is _sort_ of correct. This works because I'm doing parseInt(input.value) || 0. This makes NaN fallback to 0 if the number isn't recognized. This may require some documentation updating on best practices (or we should talk about it).
before:
after:
Do not "expand" values
This happens because parseFloat will parse "3e1" into 30. React did comparison based on string values, and "3e1" is always different than "30":
before:
after:
Pretty neat! Anything else I've missed?
nhunzaker
on 29 Jul 2016
I would suggest going one of two ways:
1) Treat value and onChange just like with a text input. Leave it up to the developer to validate input, format, and prevent invalid numbers, etc. I've created all the validators I want and everything works great for a text input, but then I don't get the number pad on mobile...
2) Prevent invalid input and let the developer handle the edge cases for "12e", "-", and ".". Meanwhile, do not allow multiple ".", "-", or "e". And enforce "-" must always be at the beginning. This is effectively what I've created with my logic for the text input.
ccorcos
on 21 Sep 2016
What is the current state of this issue?
martin-svk
on 18 Nov 2016
I have an outstanding PR here:
https://github.com/facebook/react/pull/7359
Looks like it's now in conflict with master. I'll address those conflicts.
Update: Sorry. Looks like something on CI. Upstreamed anyway.
nhunzaker
on 19 Nov 2016
Spam again... Seen a lot of this lately
On Nov 20, 2016 1:27 PM, "qagiux" [email protected] wrote:
Gonna do the first half of this in https://goo.gl/JBljdR
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/7253#issuecomment-261799117,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABJFP7sCqRcqL6lHOT7LfnoSlIIRQrJYks5rAJ8sgaJpZM4JK6Nl
.
STRML
on 20 Nov 2016
What's the state of this issue? We are still encountering this on v15.4.2
nksfrank
on 9 Mar 2017
@nksfrank I believe the plan is to ship this with 15.5:
https://github.com/facebook/react/issues/8854
nhunzaker
on 9 Mar 2017
This should be addressed in 15.5, now that we've merged the number input PR:
https://github.com/facebook/react/pull/7359#event-1017024857
I'll leave this ticket open, I don't know what the protocol is on closing tickets before a release with a fix has been published.
nhunzaker
on 27 Mar 2017
cc @aweary I just realized I can't close this :(
nhunzaker
on 28 Mar 2017
Fixed! https://jsfiddle.net/09Lxxzn9/1/
Thanks for your hard work @nhunzaker.
STRML
on 8 Apr 2017
Related issues
UnbearableBear
·
3Comments
jmullo
·
3Comments
framerate
·
3Comments
DimitarChristoff
·
3Comments
zpao
·
3Comments
Most helpful comment
Fixed! https://jsfiddle.net/09Lxxzn9/1/
Thanks for your hard work @nhunzaker.