Fix Chrome number input backspace and invalid input issue (#7359)

* Only re-assign defaultValue if it is different

* Do not set value if it is the same

* Properly cover defaultValue

* Use coercion to be smart about value assignment

* Add explanation of loose type checks in value assignment.

* Add test coverage for setAttribute update.

* Only apply loose value check to text inputs

* Fix case where empty switches to zero

* Handle zero case in controlled input

* Correct mistake with default value assignment after rebase

* Do not assign bad input to number input

* Only trigger number input value attribute updates on blur

* Remove reference to LinkedValueUtils

* Record new fiber tests

* Add tests for blurred number input behavior

* Replace onBlur wrapper with rule in ChangeEventPlugin

* Sift down to only number inputs

* Re-record fiber tests

* Add test case for updating attribute on uncontrolled inputs. Make related correction

* Handle uncontrolled inputs, integrate fiber

* Reorder boolean to mitigate DOM checks

* Only assign value if it is different

* Add number input browser test fixtures

During the course of the number input fix, we uncovered many edge
cases. This commit adds browser test fixtures for each of those instances.

* Address edge case preventing number precision lower than 1 place

0.0 coerces to 0, however they are not the same value when doing
string comparision. This prevented controlled number inputs from
inputing the characters `0.00`.

Also adds test cases.

* Accommodate lack of IE9 number input support

IE9 does not support number inputs. Number inputs in IE9 fallback to
traditional text inputs. This means that accessing `input.value` will
report the raw text, rather than parsing a numeric value.

This commit makes the ReactDOMInput wrapper check to see if the `type`
prop has been configured to `"number"`. In those cases, it will
perform a comparison based upon `parseFloat` instead of the raw input
value.

* Remove footnotes about IE exponent issues

With the recent IE9 fix, IE properly inserts `e` when it produces an
invalid number.

* Address exception in IE9/10 ChangeEventPlugin blur event

On blur, inputs have their values assigned. This is so that number
inputs do not conduct unexpected behavior in
Chrome/Safari. Unfortunately, there are cases where the target
instance might be undefined in IE9/10, raising an exception.

* Migrate over ReactDOMInput.js number input fixes to Fiber

Also re-record tests

* Update number fixtures to use latest components

* Add number input test case for dashes and negative numbers

* Replace trailing dash test case with replace with dash

Also run prettier

fixtures/dom/src/components/Header.js

@@ -0,0 +1,70 @@
+import { parse, stringify } from 'query-string';
+import getVersionTags from '../tags';
+const React = window.React;
+
+const Header = React.createClass({
+  getInitialState() {
+    const query = parse(window.location.search);
+    const version = query.version || 'local';
+    const versions = [version];
+    return { version, versions };
+  },
+  componentWillMount() {
+    getVersionTags()
+      .then(tags => {
+        let versions = tags.map(tag => tag.name.slice(1));
+        versions = [`local`, ...versions];
+        this.setState({ versions });
+      })
+  },
+  handleVersionChange(event) {
+    const query = parse(window.location.search);
+    query.version = event.target.value;
+    if (query.version === 'local') {
+      delete query.version;
+    }
+    window.location.search = stringify(query);
+  },
+  handleFixtureChange(event) {
+    window.location.pathname = event.target.value;
+  },
+  render() {
+    return (
+    <header className="header">
+      <div className="header__inner">
+        <span className="header__logo">
+          <img src="https://facebook.github.io/react/img/logo.svg" alt="" width="32" height="32" />
+          React Sandbox (v{React.version})
+        </span>
+
+        <div className="header-controls">
+          <label htmlFor="example">
+            <span className="sr-only">Select an example</span>
+            <select value={window.location.pathname} onChange={this.handleFixtureChange}>
+              <option value="/">Select a Fixture</option>
+              <option value="/range-inputs">Range Inputs</option>
+              <option value="/text-inputs">Text Inputs</option>
+              <option value="/number-inputs">Number Input</option>
+              <option value="/selects">Selects</option>
+              <option value="/textareas">Textareas</option>
+              <option value="/input-change-events">Input change events</option>
+            </select>
+          </label>
+          <label htmlFor="react_version">
+            <span className="sr-only">Select a version to test</span>
+            <select
+              value={this.state.version}
+              onChange={this.handleVersionChange}>
+                {this.state.versions.map(version => (
+                  <option key={version} value={version}>{version}</option>
+                ))}
+            </select>
+          </label>
+        </div>
+      </div>
+    </header>
+    );
+  },
+});
+
+export default Header;

fixtures/dom/src/components/fixtures/index.js

@@ -0,0 +1,34 @@
+const React = window.React;
+import RangeInputFixtures from './range-inputs';
+import TextInputFixtures from './text-inputs';
+import SelectFixtures from './selects';
+import TextAreaFixtures from './textareas';
+import InputChangeEvents from './input-change-events';
+import NumberInputFixtures from './number-inputs/';
+
+/**
+ * A simple routing component that renders the appropriate
+ * fixture based on the location pathname.
+ */
+const FixturesPage = React.createClass({
+  render() {
+    switch (window.location.pathname) {
+      case '/text-inputs':
+        return <TextInputFixtures />;
+      case '/range-inputs':
+        return <RangeInputFixtures />;
+      case '/selects':
+        return <SelectFixtures />;
+      case '/textareas':
+        return <TextAreaFixtures />;
+      case '/input-change-events':
+        return <InputChangeEvents />;
+      case '/number-inputs':
+        return <NumberInputFixtures />;
+      default:
+        return <p>Please select a test fixture.</p>;
+    }
+  },
+});
+
+module.exports = FixturesPage;

fixtures/dom/src/components/fixtures/number-inputs/NumberTestCase.js

@@ -0,0 +1,37 @@
+const React = window.React;
+
+import Fixture from '../../Fixture';
+
+const NumberTestCase = React.createClass({
+  getInitialState() {
+    return { value: '' };
+  },
+  onChange(event) {
+    const parsed = parseFloat(event.target.value, 10)
+    const value = isNaN(parsed) ? '' : parsed
+
+    this.setState({ value })
+  },
+  render() {
+    return (
+      <Fixture>
+        <div>{this.props.children}</div>
+
+        <div className="control-box">
+          <fieldset>
+            <legend>Controlled</legend>
+            <input type="number" value={this.state.value} onChange={this.onChange} />
+            <span className="hint"> Value: {JSON.stringify(this.state.value)}</span>
+          </fieldset>
+
+          <fieldset>
+            <legend>Uncontrolled</legend>
+            <input type="number" defaultValue={0.5} />
+          </fieldset>
+        </div>
+      </Fixture>
+    );
+  },
+});
+
+export default NumberTestCase;

fixtures/dom/src/components/fixtures/number-inputs/index.js

@@ -0,0 +1,167 @@
+const React = window.React;
+
+import FixtureSet from '../../FixtureSet';
+import TestCase from '../../TestCase';
+import NumberTestCase from './NumberTestCase';
+
+const NumberInputs = React.createClass({
+  render() {
+    return (
+      <FixtureSet
+          title="Number inputs"
+          description="Number inputs inconsistently assign and report the value
+                       property depending on the browser."
+      >
+        <TestCase
+            title="Backspacing"
+            description="The decimal place should not be lost"
+        >
+          <TestCase.Steps>
+            <li>Type "3.1"</li>
+            <li>Press backspace, eliminating the "1"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "3.", preserving the decimal place
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+
+          <p className="footnote">
+            <b>Notes:</b> Chrome and Safari clear trailing
+            decimals on blur. React makes this concession so that the
+            value attribute remains in sync with the value property.
+          </p>
+        </TestCase>
+
+        <TestCase
+            title="Decimal precision"
+            description="Supports decimal precision greater than 2 places"
+        >
+          <TestCase.Steps>
+            <li>Type "0.01"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "0.01"
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Exponent form"
+            description="Supports exponent form ('2e4')"
+        >
+          <TestCase.Steps>
+            <li>Type "2e"</li>
+            <li>Type 4, to read "2e4"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "2e4". The parsed value should read "20000"
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Exponent Form"
+            description="Pressing 'e' at the end"
+        >
+          <TestCase.Steps>
+            <li>Type "3.14"</li>
+            <li>Press "e", so that the input reads "3.14e"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "3.14e", the parsed value should be empty
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Exponent Form"
+            description="Supports pressing 'ee' in the middle of a number"
+        >
+          <TestCase.Steps>
+            <li>Type "3.14"</li>
+            <li>Move the text cursor to after the decimal place</li>
+            <li>Press "e" twice, so that the value reads "3.ee14"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "3.ee14"
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Trailing Zeroes"
+            description="Typing '3.0' preserves the trailing zero"
+        >
+          <TestCase.Steps>
+            <li>Type "3.0"</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "3.0"
+          </TestCase.ExpectedResult>
+
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Inserting decimals precision"
+            description="Inserting '.' in to '300' maintains the trailing zeroes"
+        >
+          <TestCase.Steps>
+            <li>Type "300"</li>
+            <li>Move the cursor to after the "3"</li>
+            <li>Type "."</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "3.00", not "3"
+          </TestCase.ExpectedResult>
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Replacing numbers with -"
+            description="Replacing a number with the '-' sign should not clear the value"
+        >
+          <TestCase.Steps>
+            <li>Type "3"</li>
+            <li>Select the entire value"</li>
+            <li>Type '-' to replace '3' with '-'</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "-", not be blank.
+          </TestCase.ExpectedResult>
+          <NumberTestCase />
+        </TestCase>
+
+        <TestCase
+            title="Negative numbers"
+            description="Typing minus when inserting a negative number should work"
+        >
+          <TestCase.Steps>
+            <li>Type "-"</li>
+            <li>Type '3'</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            The field should read "-3".
+          </TestCase.ExpectedResult>
+          <NumberTestCase />
+        </TestCase>
+      </FixtureSet>
+    );
+  },
+});
+
+export default NumberInputs;

src/renderers/dom/client/eventPlugins/ChangeEventPlugin.js

@@ -317,6 +317,26 @@ function getTargetInstForClickEvent(
   }
 }
 
+function handleControlledInputBlur(inst, node) {
+  // TODO: In IE, inst is occasionally null. Why?
+  if (inst == null) {
+    return;
+  }
+
+  // Fiber and ReactDOM keep wrapper state in separate places
+  let state = inst._wrapperState || node._wrapperState;
+
+  if (!state || !state.controlled || node.type !== 'number') {
+    return;
+  }
+
+  // If controlled, assign the value attribute to the current value on blur
+  let value = '' + node.value;
+  if (node.getAttribute('value') !== value) {
+    node.setAttribute('value', value);
+  }
+}
+
 /**
  * This plugin creates an `onChange` event that normalizes change events
  * across form elements. This event fires at a time when it's possible to
@@ -380,6 +400,11 @@ var ChangeEventPlugin = {
         targetInst
       );
     }
+
+    // When blurring, set the value attribute for number inputs
+    if (topLevelType === 'topBlur') {
+      handleControlledInputBlur(targetInst, targetNode);
+    }
   },
 
 };

src/renderers/dom/client/wrappers/ReactDOMInput.js

@@ -149,11 +149,8 @@ var ReactDOMInput = {
       initialValue: props.value != null ? props.value : defaultValue,
       listeners: null,
       onChange: _handleChange.bind(inst),
+      controlled: isControlled(props),
     };
-
-    if (__DEV__) {
-      inst._wrapperState.controlled = isControlled(props);
-    }
   },
 
   updateWrapper: function(inst) {
@@ -202,14 +199,24 @@ var ReactDOMInput = {
     var node = ReactDOMComponentTree.getNodeFromInstance(inst);
     var value = LinkedValueUtils.getValue(props);
     if (value != null) {
+      if (value === 0 && node.value === '') {
+        node.value = '0';
+        // Note: IE9 reports a number inputs as 'text', so check props instead.
+      } else if (props.type === 'number') {
+        // Simulate `input.valueAsNumber`. IE9 does not support it
+        var valueAsNumber = parseFloat(node.value, 10) || 0;
 
-      // Cast `value` to a string to ensure the value is set correctly. While
-      // browsers typically do this as necessary, jsdom doesn't.
-      var newValue = '' + value;
-
-      // To avoid side effects (such as losing text selection), only set value if changed
-      if (newValue !== node.value) {
-        node.value = newValue;
+        // eslint-disable-next-line
+        if (value != valueAsNumber) {
+          // Cast `value` to a string to ensure the value is set correctly. While
+          // browsers typically do this as necessary, jsdom doesn't.
+          node.value = '' + value;
+        }
+        // eslint-disable-next-line
+      } else if (value != node.value) {
+        // Cast `value` to a string to ensure the value is set correctly. While
+        // browsers typically do this as necessary, jsdom doesn't.
+        node.value = '' + value;
       }
     } else {
       if (props.value == null && props.defaultValue != null) {

src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js

@@ -268,6 +268,48 @@ describe('ReactDOMInput', () => {
     expect(node.value).toBe('0');
   });
 
+  it('should properly control 0.0 for a text input', () => {
+    var stub = <input type="text" value={0} onChange={emptyFunction} />;
+    stub = ReactTestUtils.renderIntoDocument(stub);
+    var node = ReactDOM.findDOMNode(stub);
+
+    node.value = '0.0';
+    ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}});
+    expect(node.value).toBe('0.0');
+  });
+
+  it('should properly control 0.0 for a number input', () => {
+    var stub = <input type="number" value={0} onChange={emptyFunction} />;
+    stub = ReactTestUtils.renderIntoDocument(stub);
+    var node = ReactDOM.findDOMNode(stub);
+
+    node.value = '0.0';
+    ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}});
+    expect(node.value).toBe('0.0');
+  });
+
+  it('should properly transition from an empty value to 0', function() {
+    var container = document.createElement('div');
+
+    ReactDOM.render(<input type="text" value="" />, container);
+    ReactDOM.render(<input type="text" value={0} />, container);
+
+    var node = container.firstChild;
+
+    expect(node.value).toBe('0');
+  });
+
+  it('should properly transition from 0 to an empty value', function() {
+    var container = document.createElement('div');
+
+    ReactDOM.render(<input type="text" value={0} />, container);
+    ReactDOM.render(<input type="text" value="" />, container);
+
+    var node = container.firstChild;
+
+    expect(node.value).toBe('');
+  });
+
   it('should have the correct target value', () => {
     var handled = false;
     var handler = function(event) {
@@ -937,4 +979,88 @@ describe('ReactDOMInput', () => {
       'node.setAttribute("checked", "")',
     ]);
   });
+
+  describe('assigning the value attribute on controlled inputs', function() {
+    function getTestInput() {
+      return React.createClass({
+        getInitialState: function() {
+          return {
+            value: this.props.value == null ? '' : this.props.value,
+          };
+        },
+        onChange: function(event) {
+          this.setState({value: event.target.value});
+        },
+        render: function() {
+          var type = this.props.type;
+          var value = this.state.value;
+
+          return <input type={type} value={value} onChange={this.onChange} />;
+        },
+      });
+    }
+
+    it('always sets the attribute when values change on text inputs', function() {
+      var Input = getTestInput();
+      var stub = ReactTestUtils.renderIntoDocument(<Input type="text" />);
+      var node = ReactDOM.findDOMNode(stub);
+
+      ReactTestUtils.Simulate.change(node, {target: {value: '2'}});
+
+      expect(node.getAttribute('value')).toBe('2');
+    });
+
+    it('does not set the value attribute on number inputs if focused', () => {
+      var Input = getTestInput();
+      var stub = ReactTestUtils.renderIntoDocument(
+        <Input type="number" value="1" />,
+      );
+      var node = ReactDOM.findDOMNode(stub);
+
+      node.focus();
+
+      ReactTestUtils.Simulate.change(node, {target: {value: '2'}});
+
+      expect(node.getAttribute('value')).toBe('1');
+    });
+
+    it('sets the value attribute on number inputs on blur', () => {
+      var Input = getTestInput();
+      var stub = ReactTestUtils.renderIntoDocument(
+        <Input type="number" value="1" />,
+      );
+      var node = ReactDOM.findDOMNode(stub);
+
+      ReactTestUtils.Simulate.change(node, {target: {value: '2'}});
+      ReactTestUtils.SimulateNative.blur(node);
+
+      expect(node.getAttribute('value')).toBe('2');
+    });
+
+    it('an uncontrolled number input will not update the value attribute on blur', () => {
+      var stub = ReactTestUtils.renderIntoDocument(
+        <input type="number" defaultValue="1" />,
+      );
+      var node = ReactDOM.findDOMNode(stub);
+
+      node.value = 4;
+
+      ReactTestUtils.SimulateNative.blur(node);
+
+      expect(node.getAttribute('value')).toBe('1');
+    });
+
+    it('an uncontrolled text input will not update the value attribute on blur', () => {
+      var stub = ReactTestUtils.renderIntoDocument(
+        <input type="text" defaultValue="1" />,
+      );
+      var node = ReactDOM.findDOMNode(stub);
+
+      node.value = 4;
+
+      ReactTestUtils.SimulateNative.blur(node);
+
+      expect(node.getAttribute('value')).toBe('1');
+    });
+  });
 });

src/renderers/dom/shared/HTMLDOMPropertyConfig.js

@@ -210,7 +210,34 @@ var HTMLDOMPropertyConfig = {
     htmlFor: 'for',
     httpEquiv: 'http-equiv',
   },
-  DOMPropertyNames: {
+  DOMPropertyNames: {},
+  DOMMutationMethods: {
+    value: function(node, value) {
+      if (value == null) {
+        return node.removeAttribute('value');
+      }
+
+      // Number inputs get special treatment due to some edge cases in
+      // Chrome. Let everything else assign the value attribute as normal.
+      // https://github.com/facebook/react/issues/7253#issuecomment-236074326
+      if (node.type !== 'number' || node.hasAttribute('value') === false) {
+        node.setAttribute('value', '' + value);
+      } else if (
+        node.validity &&
+        !node.validity.badInput &&
+        node.ownerDocument.activeElement !== node
+      ) {
+        // Don't assign an attribute if validation reports bad
+        // input. Chrome will clear the value. Additionally, don't
+        // operate on inputs that have focus, otherwise Chrome might
+        // strip off trailing decimal places and cause the user's
+        // cursor position to jump to the beginning of the input.
+        //
+        // In ReactDOMInput, we have an onBlur event that will trigger
+        // this function again when focus is lost.
+        node.setAttribute('value', '' + value);
+      }
+    },
   },
 };
 

src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

@@ -348,6 +348,35 @@ describe('DOMPropertyOperations', () => {
 
   });
 
+  describe('value mutation method', function() {
+    it('should update an empty attribute to zero', function() {
+      var stubNode = document.createElement('input');
+      var stubInstance = {_debugID: 1};
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
+
+      stubNode.setAttribute('type', 'radio');
+
+      DOMPropertyOperations.setValueForProperty(stubNode, 'value', '');
+      spyOn(stubNode, 'setAttribute');
+      DOMPropertyOperations.setValueForProperty(stubNode, 'value', 0);
+
+      expect(stubNode.setAttribute.calls.count()).toBe(1);
+    });
+
+    it('should always assign the value attribute for non-inputs', function() {
+      var stubNode = document.createElement('progress');
+      var stubInstance = {_debugID: 1};
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
+
+      spyOn(stubNode, 'setAttribute');
+
+      DOMPropertyOperations.setValueForProperty(stubNode, 'value', 30);
+      DOMPropertyOperations.setValueForProperty(stubNode, 'value', '30');
+
+      expect(stubNode.setAttribute.calls.count()).toBe(2);
+    });
+  });
+
   describe('deleteValueForProperty', () => {
     var stubNode;
     var stubInstance;