Closed Bug 1642570 Opened 3 years ago Closed 4 months ago

Form History isn't saved for input fields with `defaultValue === value`

Categories

(Toolkit :: Form Manager, defect, P1)

76 Branch
defect

Tracking

()

RESOLVED FIXED
105 Branch
Webcompat Priority P2
Tracking Status
firefox105 --- fixed

People

(Reporter: eddyw.dev, Assigned: serg)

References

(Blocks 1 open bug)

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.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Form Autofill
Product: Firefox → Toolkit

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?

Blocks: 463486
Status: UNCONFIRMED → NEW
Component: Form Autofill → Form Manager
Ever confirmed: true
Summary: Autofill/Autocomplete isn't saved for input fields with `defaultValue === value` → Form History isn't saved for input fields with `defaultValue === value`

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)

  1. 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.
  2. 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.
  3. 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.
  1. React can handle inputs in two ways
  • uncontrolled inputs: do not have state (you can read from node.value directly to get the value)
  • controlled inputs: these have state (the value of the input lives in React state, so node.value isn't the source of truth)
    If React didn't assign defaultValue = 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.
  1. 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 if value === defaultValue

  2. 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.

The severity field is not set for this bug.
:MattN, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(MattN+bmo)

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.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(MattN+bmo)
Resolution: --- → WONTFIX

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.

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Webcompat Priority: --- → ?
Webcompat Priority: ? → P2

Hi Sergey, wonder if someone could take a look at this? It looks like this problem is affecting a lot of web developers.

Flags: needinfo?(sgalich)
Severity: -- → S3
Depends on: 1771806
Flags: needinfo?(sgalich)
Priority: -- → P3
Priority: P3 → P1
Assignee: nobody → jneuberger

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.

(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?".

Assignee: jneuberger → nobody
Assignee: nobody → sgalich

Bug 589471 and 1481802 may be fixed while improving testing for this bug.

Blocks: 589471, 1481802
Blocks: 543322
Attachment #9284078 - Attachment description: WIP: Bug 1642570 - Form History isn't saved for input fields with `defaultValue === value` → Bug 1642570 - capture only user input for Form History
Attachment #9284078 - Attachment description: Bug 1642570 - capture only user input for Form History → WIP: Bug 1642570 - capture only user input for Form History
Attachment #9284078 - Attachment description: WIP: Bug 1642570 - capture only user input for Form History → Bug 1642570 - capture only user input for Form History r=dimi!
Blocks: 1779967
Pushed by sgalich@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/592b0b4a8424
capture only user input for Form History r=dimi

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

Flags: needinfo?(sgalich)

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.

Flags: needinfo?(sgalich)
Pushed by sgalich@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c4aa8e6fa0c
capture only user input for Form History r=dimi
Attachment #9284078 - Attachment description: Bug 1642570 - capture only user input for Form History r=dimi! → WIP: Bug 1642570 - capture only user input for Form History r=dimi!
Attachment #9284078 - Attachment description: WIP: Bug 1642570 - capture only user input for Form History r=dimi! → Bug 1642570 - capture only user input for Form History r=dimi!
Pushed by sgalich@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/847b534dea9b
capture only user input for Form History r=dimi
Flags: needinfo?(sgalich)
Status: REOPENED → RESOLVED
Closed: 2 years ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
Blocks: 1782132
Regressions: 1783102
Blocks: 578879
You need to log in before you can comment on or make changes to this bug.