Form History isn't saved for input fields with `defaultValue === value`
Categories
(Toolkit :: Form Manager, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox105 | --- | fixed |
People
(Reporter: eddyw.dev, Assigned: serg)
References
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.138 Safari/537.36
Steps to reproduce:
Autofill isn't saved when an input DOM node has node.defaultValue === node.value
when submitted.
Any ReactJS app that uses controlled inputs,
Here is an example: https://codesandbox.io/embed/material-demo-y5mlt
Here is another example with plain HTML and 3 line of JS code that updates the node.defaultValue
: https://codesandbox.io/s/naughty-feather-vwl6v?file=/index.html (over-simplification of what ReactJS does under the hood on user type)
The relevant issue in ReactJS repo: https://github.com/facebook/react/issues/18986
Actual results:
All ReactJS applications that use "controlled inputs" are affected. ReactJS updates the input DOM node's defaultValue
to be equal to value
as the user types. When the form is submitted, Firefox doesn't keep/save the form values for autocomplete/autofill.
In ReactJS web apps, this is an issue because it ruins user experience. ReactJS does this for "controlled inputs" so when a form is reset, it won't make all fields blank. Also, as far as I know, Firefox doesn't trigger events on inputs when form is reset, so it's still an issue and the best workaround still seems to be to do node.defaultValue = node.value
so the state isn't out of sync with the form. However, this workaround introduces a new problem that only happens in Firefox, it bails out of saving input values for autofill/autocomplete.
Expected results:
Submitting a form where input DOM nodes have input.defaultValue === input.value
should still save them for autofill.
Why? because the defaultValue
of an input may have come from a different source other than autocomplete.
For instance, a user profile was saved in Google Chrome, but the user opened it in Firefox and tried to modify their profile in Firefox (all fields with defaultValue
). In this case, Firefox would only save one field for autofill/autocomplete rather than all of them. Chrome would offer autocomplete for all fields but Firefox only for one of them which is pretty confusing. If this same form was written in ReactJS with "controlled inputs", Firefox wouldn't offer autofill at all.
Comment 1•4 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Comment 2•4 years ago
|
||
Thanks for filing. Just some initial comments for now:
(In reply to eddyw.dev from comment #0)
Autofill isn't saved when an input DOM node has
node.defaultValue === node.value
when submitted.
Right, this is intentional. This was implemented 11 years ago (bug 463486) and I don't see bugs associated with that in all these years…
In ReactJS web apps, this is an issue because it ruins user experience. ReactJS does this for "controlled inputs" so when a form is reset, it won't make all fields blank. Also, as far as I know, Firefox doesn't trigger events on inputs when form is reset, so it's still an issue
Not sure I follow but does https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/reset_event not meet your needs?
Firefox doesn't trigger events on inputs when form is reset
Maybe I should have phrased this better. This isn't a Firefox-only thing, but there is no onreset
for input fields. Also input
& change
events aren't triggered when a form is reset. Sure, you could use reset
event on the form and preventDefault
. However, that's not the same as having the inputs with value === defaultValue
and resetting the form. In the case of React, the difference is that "controlled inputs" would not seem to reset (because they have value === defaultValue
). However, "uncontrolled inputs" would be reset.
Anyways, this is still an issue. Quoting from the mentioned issue (bug 463486):
Default/unmodified form field values shouldn't be remembered by form manager. If the user didn't enter the text, it seems odd to remember it. This may also help cut down the volume of form data that's remembered.
Modifying an input DOM node.value
and node.defaultValue
while user is typing doesn't mean the form field values are unmodified or that the user didn't enter any text. node.defaultValue
is a mutable property after all (setter/getter)
Is the volume of form data a problem? (I mean, after 11 years since this was first implemented)
I don't see bugs associated with that in all these years…
To be fair, I have always had autocomplete/autofill disabled, and React is younger than that. There are people who reported issues in the wrong places regarding the behavior of autofill/autocomplete in Firefox, that's maybe why you haven't seen them:
https://github.com/ant-design/ant-design/issues/21232
https://github.com/mui-org/material-ui/issues/16943
https://github.com/facebook/react/issues/17022
https://stackoverflow.com/questions/48645775/autofill-doesnt-work-for-firefox/48716623 (comment mentions its a React component)
Comment 4•4 years ago
|
||
- Could you explain more about why the
reset
isn't enough for React and what React needs? It's not yet clear to me why this isn't a React issue. - Do you happen to know how other browsers handle this? I can't think of a straightforward way to determine whether a value was user-edited when you have JS code manipulating .value and .defaultValue.
- What kind of data are you submitting in fields? Address (including names/emails/phone numbers), credit cards, usernames/passwords or other data? Each of these go through different components of Firefox and can handle .defaultValue differently because some have more context on the purpose of the field. For example, if we know that a field is an email address as part of an address form then we may not care about ignoring when
.value == .defaultValue
since the value is still likely useful to the user. Whereas arbitrary form values pre-filled by the site are probably less useful. I realize that you may want this to work on all fields but saving all field values can be a worse experience than this bug which only affects React.
- React can handle inputs in two ways
- uncontrolled inputs: do not have state (you can read from
node.value
directly to get thevalue
) - controlled inputs: these have state (the
value
of the input lives in React state, sonode.value
isn't the source of truth)
If React didn't assigndefaultValue = value
, a form reset would make these inputs blank. So you'd have state out of sync with the input's value. This is usually how inputs are handled in React. It's because of this that autofill just doesn't work.
-
From what I can tell, other browsers just check if the value is in history (previously saved) or not and ignore completely
defaultValue
. So basically, they don't compare ifvalue === defaultValue
-
I haven't tested with all kinds of fields but names, emails, and addresses
Whereas arbitrary form values pre-filled by the site are probably less useful.
Take for example that a user fills their .. profile (or whatever) in a different browser:
(silly form just for demonstration)
<input name="fullname" value="Bob" defaultValue="" />
<input name="favorite_color" value="Red" defaultValue="" />
Then user decides to open their profile in Firefox and this is what they get:
<input name="fullname" value="Bob" defaultValue="Bob" />
<input name="favorite_color" value="Red" defaultValue="Red" />
If user updates in Firefox the input favorite_color
to hotpink
and submit, if it's not a React app, you'd have autofill only for favorite_color
(because favorite_color.value !== favorite_color.defaultValue
) .. which some would argue that it's the less useful value in this case ...and no autofill for fullname
However, if a user filled their profile first in Firefox (and not on that different browser), you'd get all fields saved for autofill because they'd have defaultValue=""
at first.
That is the confusing behavior. It happens to affect React badly but the issue isn't just about broken autofill in React apps but that Firefox takes defaultValue
as a reliable source of truth and it's very inconsistent on how & when it decides to save the values (as shown in example which depends on where/which browser you filled it first)
Maybe there could a setting in Firefox to make autofill less greedy and save all fields (ignore defaultValue
)? After all, a developer could choose to make an input autocomplete="off"
if its value is not important which is more explicit and it can tell reliably to Firefox that the value doesn't matter for autofill.
Comment 6•4 years ago
|
||
The severity field is not set for this bug.
:MattN, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 7•4 years ago
|
||
Hello, I don't think React is honouring the intent of intent of the defaultValue
attribute… it's supposed to represent the default value before the user edited (which is also what the field would get reset to). Since this primarily affects React, and they acknowledge it's a bug, I don't think we will change this long-standing behaviour. This seems like something the React teams plans to fix in https://github.com/facebook/react/issues/15739
Thanks for the report.
Comment 8•2 years ago
|
||
This seems to be the underlying issue that's causing people to try to do feature detection using InstallTrigger in https://bugzilla.mozilla.org/show_bug.cgi?id=1754441#c29. Given that react appear to be unwilling to change their (admittedly very odd!) behaviour, and this is causing real problems for authors who are trying to support Firefox (and likely users, when the authors are less diligent), I think we should consider aligning with Blink/WebKit behaviour here in order to avoid compatibility issues.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 9•2 years ago
|
||
Hi Sergey, wonder if someone could take a look at this? It looks like this problem is affecting a lot of web developers.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 10•2 years ago
•
|
||
I think removing this intentional check might not be the right way to deal with this bug.
I did a test on both Safari and Chrome, both don't save input to form history when value == defaultValue
, however, it seems that Chrome does something extra. In my test, if you modify the defaultValue
on the fly (which is what react is doing now?), after submitting the form, Chrome compares the value with the initial defaultValue
, not the defaultValue
updated by the script afterwards. The behavior make it look like Chrome doesn't compare value == defaultValue
, but actually it does.
I test this example: https://codesandbox.io/embed/material-demo-y5mlt
Safari has the same behavior with us. Chrome doesn't have this issue probably because the trick mentioned above.
In summary, removing the value == defaultValue
will make us behave differently with other browsers. In my opinion, it won't be a better design either. If this is something we need to fix, I'd suggest using a workaround to address this special case and not change the general behavior.
Assignee | ||
Comment 11•2 years ago
|
||
(In reply to Dimi Lee [:dimi][:dlee] from comment #10)
I test this example: https://codesandbox.io/embed/material-demo-y5mlt
Safari has the same behavior with us. Chrome doesn't have this issue probably because the mentioned trick above.
Thank you for checking Safari and confirming it's not just Firefox that has this issue.
I've looked at using mLastValueChangeWasInteractive but it didn't help because last interaction is done by a script.
I think we can't avoid but focus on bug 1771806 first, it would solve the problem of "did user enter this value?".
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 12•2 years ago
|
||
Assignee | ||
Comment 13•2 years ago
|
||
Bug 589471 and 1481802 may be fixed while improving testing for this bug.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 14•2 years ago
|
||
Comment 15•2 years ago
•
|
||
Backed out changeset 592b0b4a8424 (Bug 1642570) for causing high freq mochitest failures on test_popup_enter_event.html.
Backout link
Push with failures <--> 14
14 Failure Log
6 Failure Log
Assignee | ||
Comment 16•2 years ago
|
||
Thanks Marian-Vasile Laza.
The failing test was not related to the changes in this patch, lets try landing again.
P.S. I'll take a look at that failing test in another patch as part of Bug 1309654, it does need some upgrade.
Comment 17•2 years ago
|
||
Comment 18•2 years ago
•
|
||
Backed out for causing failures at test_popup_enter_event.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/6df2ad672424e1f142ce0b918170bbd29473767b
Failure log: https://treeherder.mozilla.org/logviewer?job_id=385055203&repo=autoland&lineNumber=9977
Updated•2 years ago
|
Updated•2 years ago
|
Comment 19•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Comment 20•2 years ago
|
||
bugherder |
Assignee | ||
Updated•2 years ago
|
Comment hidden (Intermittent Failures Robot) |
Description
•