Skip to content

Commit

Permalink
Avoid setting empty value on reset & submit inputs
Browse files Browse the repository at this point in the history
  • Loading branch information
ellsclytn committed Aug 3, 2018
1 parent 261da3f commit 7b1047c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
33 changes: 29 additions & 4 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Expand Up @@ -752,15 +752,40 @@ describe('ReactDOMInput', () => {

it('should not set a value for submit buttons unnecessarily', () => {
const stub = <input type="submit" />;
const node = ReactDOM.render(stub, container);
ReactDOM.render(stub, container);
const node = container.firstChild;

// The value shouldn't be '', or else the button will have no text; it
// should have the default "Submit" or "Submit Query" label. Most browsers
// report this as not having a `value` attribute at all; IE reports it as
// the actual label that the user sees.
expect(
!node.hasAttribute('value') || node.getAttribute('value').length > 0,
).toBe(true);
expect(node.hasAttribute('value')).toBe(false);
});

it('should not set a value for submit buttons when undefined is supplied', () => {
let stub = <input type="submit" onChange={emptyFunction} />;
ReactDOM.render(stub, container);
stub = <input type="submit" value={undefined} onChange={emptyFunction} />;
ReactDOM.render(stub, container);
const node = container.firstChild;

expect(node.hasAttribute('value')).toBe(false);
});

it('should set a value to a submit input', () => {
let stub = <input type="submit" value="banana" />;
ReactDOM.render(stub, container);
const node = container.firstChild;

expect(node.getAttribute('value')).toBe('banana');
});

it('should set an empty string value on a submit input', () => {
let stub = <input type="submit" value="" />;
ReactDOM.render(stub, container);
const node = container.firstChild;

expect(node.getAttribute('value')).toBe('');
});

it('should control radio buttons', () => {
Expand Down
18 changes: 18 additions & 0 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Expand Up @@ -182,6 +182,14 @@ export function updateWrapper(element: Element, props: Object) {
} else if (node.value !== '' + value) {
node.value = '' + value;
}
} else if (
(props.type === 'submit' || props.type === 'reset') &&
(value === null || value === undefined)
) {
// Submit/reset inputs need the attribute removed completely to avoid
// blank-text buttons.
node.removeAttribute('value');
return;
}

if (props.hasOwnProperty('value')) {
Expand All @@ -203,6 +211,16 @@ export function postMountWrapper(
const node = ((element: any): InputWithWrapperState);

if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
// Submit or reset inputs should not have their value overridden
// with a null or undefined value, or else this will result in a button with
// no text, ignoring the browser-default "Submit", etc.
if (
(props.value === undefined || props.value === null) &&
(props.type === 'submit' || props.type === 'reset')
) {
return;
}

const initialValue = '' + node._wrapperState.initialValue;
const currentValue = node.value;

Expand Down

0 comments on commit 7b1047c

Please sign in to comment.