Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust input value detachment to avoid input validation in FireFox #11202

Closed
wants to merge 4 commits into from

Conversation

nhunzaker
Copy link
Contributor

Fixes #8395

React DOM intentionally sets value on inputs when they are created. This prevents an issue where changes to defaultValue cause the value property to change.

This is colloquially known as "value detachment". Unfortunately, this triggers input validation, displaying a red aura in Firefox. This works around that by:

  1. Only detaching if the related value is truthy.
  2. Reassigning the type property on date/color inputs to preserve
    the iOS fix.

Test Plan

  1. Open http://react-ff-validate.surge.sh/text-inputs
  2. The Required Inputs test case inputs should not be invalid (no red)
  3. The date input placeholders on the All inputs test case should display

So far, I have tested in:

Success so far:
Safari Desktop 11
Safari iOS 7, 8.4, 10.2, 11
Firefox 47, 56
Chrome 61
IE 10, 11
IE Edge 15

React DOM intentionally sets `value` on inputs when they are
created. This prevents an issue where changes to `defaultValue` cause
the `value` property to change.

This is colloquially known as "value detachment". Unfortunately, this
triggers input validation, displaying a red aura in Firefox. This
works around that by:

1. Only detaching if the related value is truthy.
2. Reassigning the `type` property on date/color inputs to preserve
   the iOS fix.
//
// Important: use setAttribute instead of node.type = "x" to avoid
// an exception in IE10/11 due to an unrecognized input type
node.setAttribute('type', 'text');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that you can't change the type of an input after it's created in at least some versions of IE. Do you mind double checking which to be sure we don't break something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely true for some browsers for password types, for "security".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a quick test:
https://codepen.io/nhunzaker/full/BwPbRK/

This works in:

  • IE 9, 10, 11
  • Firefox 47
  • Chrome 41
  • Safari Desktop 7.0
  • Safari iOS 9

Is this a reasonable test case? This page is over HTTPs. I wonder if it would be different otherwise.

In any case, this only applies to date/time/color inputs, and only when they are mounted for the first time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, sorry. Key detail! This fails in IE8, but not IE9 or higher:

screen shot 2017-10-12 at 5 50 48 pm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this should be OK then.

Long-term I suppose someone could make an IE8-compatible renderer using the react-reconciler package if they wanted to.

@nhunzaker
Copy link
Contributor Author

Hmm. I'm hitting the following error trying to run prettier:

Error: spawnSync /Users/nate/Development/react/node_modules/.bin/prettier E2BIG

I guess there are too many arguments to prettier. Hmm...

@nhunzaker
Copy link
Contributor Author

🤦‍♂️ Fixed. Sorry, pushed to the wrong remote 🙄.

@nhunzaker
Copy link
Contributor Author

This was invalidated by #11746, but the fixture is still important. I'll send a follow-up PR regarding that fixture.

@nhunzaker nhunzaker closed this Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firefox validation triggers on input component render
4 participants