Skip to content

Commit

Permalink
Do not assign node.value on input creation if no change will occur
Browse files Browse the repository at this point in the history
This commit fixes an issue where assigning an empty string to required
text inputs triggers the invalid state in Firefox (~60.0.1).

It does this by first comparing the initial state value to the current
value property on the text element. This:

1. Prevents the validation issue
2. Avoids an extra DOM Mutation in some cases
  • Loading branch information
nhunzaker committed May 29, 2018
1 parent aa85b0f commit b7c136d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
23 changes: 19 additions & 4 deletions fixtures/dom/src/components/fixtures/text-inputs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,36 @@ class TextInputFixtures extends React.Component {
<Fixture>
<form className="control-box">
<fieldset>
<legend>Text</legend>
<legend>Empty value prop string</legend>
<input value="" required={true} />
</fieldset>
<fieldset>
<legend>No value prop</legend>
<input required={true} />
</fieldset>
<fieldset>
<legend>Date</legend>
<legend>Empty defaultValue prop string</legend>
<input required={true} defaultValue="" />
</fieldset>
<fieldset>
<legend>No value prop date input</legend>
<input type="date" required={true} />
</fieldset>
<fieldset>
<legend>Empty value prop date input</legend>
<input type="date" value="" required={true} />
</fieldset>
</form>
</Fixture>

<p className="footnote">
Checking the date type is also important because of a prior fix for
iOS Safari that involved assigning over value/defaultValue
properties of the input to prevent a display bug. This also
triggered input validation.
properties of the input to prevent a display bug. This also triggers
input validation.
</p>
<p className="footnote">
The date inputs should be blank in iOS. This is not a bug.
</p>
</TestCase>

Expand Down
14 changes: 11 additions & 3 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,24 @@ export function postMountWrapper(element: Element, props: Object) {
const node = ((element: any): InputWithWrapperState);

if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
const initialValue = '' + node._wrapperState.initialValue;
const currentValue = node.value;

// Do not assign value if it is already set. This prevents user text input
// from being lost during SSR hydration.
if (node.value === '') {
node.value = '' + node._wrapperState.initialValue;
if (currentValue === '') {
// Do not re-assign the value property if there is no change. This
// potentially avoids a DOM write and prevents Firefox (~60.0.1) from
// prematurely marking required inputs as invalid
if (initialValue !== currentValue) {
node.value = initialValue;
}
}

// value must be assigned before defaultValue. This fixes an issue where the
// visually displayed value of date inputs disappears on mobile Safari and Chrome:
// https://github.com/facebook/react/issues/7233
node.defaultValue = '' + node._wrapperState.initialValue;
node.defaultValue = initialValue;
}

// Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
Expand Down

0 comments on commit b7c136d

Please sign in to comment.