[TextField] Handle Chrome autofill #17436

croraf · 2019-09-15T15:49:04Z

[ X ] I have followed (at least) the PR section of the contributing guide.

Fixes #14427 as per given fix by @oliviertassinari

mui-pr-bot · 2019-09-15T15:57:27Z

Details of bundle changes.

Comparing: e9f26fc...1e77b3f

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	+0.07% 🔺	+0.08% 🔺	331,688	331,926	90,532	90,607
@material-ui/core/Paper	0.00%	0.00%	68,659	68,659	20,458	20,458
@material-ui/core/Paper.esm	0.00%	0.00%	62,098	62,098	19,203	19,203
@material-ui/core/Popper	0.00%	0.00%	28,397	28,397	10,159	10,159
@material-ui/core/Textarea	0.00%	0.00%	5,082	5,082	2,132	2,132
@material-ui/core/TrapFocus	0.00%	0.00%	3,766	3,766	1,596	1,596
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	16,348	16,348	5,818	5,818
@material-ui/core/useMediaQuery	0.00%	0.00%	2,488	2,488	1,050	1,050
@material-ui/lab	0.00%	0.00%	154,749	154,749	46,885	46,885
@material-ui/styles	0.00%	0.00%	51,420	51,420	15,288	15,288
@material-ui/system	0.00%	0.00%	15,581	15,581	4,351	4,351
Button	0.00%	0.00%	78,610	78,610	24,024	24,024
Modal	0.00%	0.00%	14,542	14,542	5,114	5,114
Portal	0.00%	0.00%	2,907	2,907	1,318	1,318
Rating	0.00%	0.00%	69,963	69,963	21,856	21,856
Slider	0.00%	0.00%	75,084	75,084	23,184	23,184
colorManipulator	0.00%	0.00%	3,835	3,835	1,519	1,519
docs.landing	0.00%	0.00%	52,232	52,232	13,768	13,768
docs.main	+0.03% 🔺	+0.05% 🔺	597,197	597,393	190,673	190,763
packages/material-ui/build/umd/material-ui.production.min.js	+0.08% 🔺	+0.09% 🔺	302,599	302,831	86,969	87,049

Generated by 🚫 dangerJS against 1e77b3f

eps1lon

There is no confirmed reproduction that rules out browser issues. Please include the detailed reproduction in the PR.

oliviertassinari · 2019-09-16T08:19:13Z

The changes should solve two structural issues: 1. the JavaScript change event was fired before we listen to it. 2. The input value is hidden for security reasons. What browser issues do you have in mind?

eps1lon · 2019-09-17T08:49:31Z

What browser issues do you have in mind?

That was my question. It is not obvious what issue is solved here. Only theoretical issues that have no confirmed reproduction.

oliviertassinari · 2019-09-17T09:00:09Z

Only theoretical issues that have no confirmed reproduction.

We have a couple of root issues:

A user writes in an input before we listen to the event. This can be reproduced on the /examples/ssr/ example with a set timeout in the hydration logic.
Chrome autofill the input before React hydrates the from. This can be reproduced with [TextField] Handle Chrome autofill #14427 (comment).
Chrome doesn't fire a change event. This can be reproduced in reproduced [TextField] Handle Chrome autofill #14427 (comment) with BrowserStack Windows 10, Chrome latest

eps1lon · 2019-09-17T09:07:01Z

To be clear: A reproduction is code + environment.

eps1lon · 2019-09-18T07:19:53Z

A user writes in an input before we listen to the event. This can be reproduced on the /examples/ssr/ example with a set timeout in the hydration logic.

What is the use case for hydrating after a timeout? Seems like a strawman to me.

Chrome autofill the input before React hydrates the from. This can be reproduced with #14427 (comment).

As I said no example in that issue has a confirmable reproduction. It might reproduce for you but without knowing the environment you cannot know if this is a library or environment issue.

Chrome doesn't fire a change event. This can be reproduced in reproduced #14427 (comment) with BrowserStack Windows 10, Chrome latest

BrowserStack is not a reliable source for reproduction. If it was we would not have tabable buttons in safari.

oliviertassinari · 2019-09-18T07:48:29Z

What is the use case for hydrating after a timeout?

For instance, you might want to wait for an optional polyfill to be loaded, over the network, before hydrating your DOM. I had the very use case at Onepixel, I was frustrated Next.js has no support for it.

As I said no example in that issue has a confirmable reproduction.

Then we can call it a nice side effect of fixing the previous issue.

BrowserStack is not a reliable source for the reproduction

Don't worry, we have enough users to report if these fixes don't cut it. I don't think that we should wait to get the perfect reproduction, we might never have it. I think that we should move fast (type 2, reversible changes) by sacrificing the scope of the problems we solve and the information we collect, without sacrificing the code quality so we can learn from "production". In this case, we have an opportunity to try these changes. Worse case 1. we still have this bug, people provide a better reproduction, worse case 2. we solved the bug, but we remove the logic in the future, for some reason, people report the regression or they don't. In any case, the changes are direct application of people reported solution to the problem we can find on GitHub issues, commit, blog post or StackOverflow. @croraf and I have seen a positive impact with them. Things are imperfect and will always be :).

eps1lon · 2019-09-18T08:10:06Z

Well thanks for adding technical debt that we need to refactor.

oliviertassinari · 2019-09-18T10:42:18Z

@eps1lon I'm sorry you feel this way. I understand that you have done great progress in the past simplifying the logic of this component and that you want to keep it in a great state. I will personally make sure this new logic only remains if proven valuable. Meaning:

If React solves the related issues Provide a way to handle browser-autocompleted form values on controlled components facebook/react#1159, Fire change events for changes made to inputs before hydration facebook/react#12955, investigate if we can remove the patch (as far as I know, these issues have been present since the first version of React).
If we still get autocomplete issue report, investigate if these changes are really relevant, look for better solutions.

But yes, the more problems we solve, the more complex the codebase is and the more inertia we have. Sometimes, it's better not to solve problems reported by our users. I don't think it's one of these cases.
Assuming that we want to solve the problem. I don't think that we went down a quick patch road. It should be resilient.

* [docs] Batch small changes (mui#17435) * [docs] Add synonyms for brand icons (mui#17455) * [docs] Add synonyms for brand icons * Remove Ic prefixed icons * [docs] Improve in-site search (mui#17450) * [ButtonBase] Fix blurry text issue (mui#17453) * [docs] Revert hits per page change (mui#17458) * [docs] Fix heading format in CONTRIBUTING.md (mui#17460) * [Chip] Load the right version of Avatar (mui#17469) * [TablePagination] Merge root classes properly (mui#17467) * [Breadcrumbs] Improve API docs (mui#17468) * [TextField] Handle Chrome autofill (mui#17436) * [docs] Clarify props spread for ListItem when button flag is set (mui#17466)

jkeruzec · 2019-09-23T13:25:37Z

this update makes me change something to still have my transparent background for my input:

Separate css

/* Chrome hack for auto set password ;) */
@-webkit-keyframes autofill {
    to {
        color: white;
        background: transparent;
    }
}

Adaptation in theming, to still have this transparent background and not a grey-blue one

MuiInputBase: {
            input: {
                '&:-webkit-autofill': {
                    animationName: 'autofill',
                    animationFillMode: 'both',
                    animationDuration: '0s'
                  },
            }
        }

oliviertassinari · 2019-09-23T13:30:36Z

@jkeruzec Do you have a reproduction?

[TextField] Handle Chrome autofill

1e77b3f

oliviertassinari added component: InputBase bug 🐛 Something doesn't work labels Sep 15, 2019

oliviertassinari approved these changes Sep 15, 2019

View reviewed changes

eps1lon requested changes Sep 16, 2019

View reviewed changes

oliviertassinari added the PR: ready to ship label Sep 17, 2019

oliviertassinari merged commit 1fbbb89 into mui:master Sep 18, 2019

ntbosscher mentioned this pull request Sep 4, 2020

[TextField] Autofill values not triggering onChange in Chrome #22488

Closed

2 tasks

voliva mentioned this pull request Oct 21, 2020

[FormControl] Maximum update depth exceeded #12197

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TextField] Handle Chrome autofill #17436

[TextField] Handle Chrome autofill #17436

croraf commented Sep 15, 2019 •

edited

mui-pr-bot commented Sep 15, 2019

eps1lon left a comment

oliviertassinari commented Sep 16, 2019

eps1lon commented Sep 17, 2019

oliviertassinari commented Sep 17, 2019 •

edited

eps1lon commented Sep 17, 2019

eps1lon commented Sep 18, 2019

oliviertassinari commented Sep 18, 2019 •

edited

eps1lon commented Sep 18, 2019

oliviertassinari commented Sep 18, 2019

jkeruzec commented Sep 23, 2019

oliviertassinari commented Sep 23, 2019 •

edited

[TextField] Handle Chrome autofill #17436

[TextField] Handle Chrome autofill #17436

Conversation

croraf commented Sep 15, 2019 • edited

mui-pr-bot commented Sep 15, 2019

eps1lon left a comment

Choose a reason for hiding this comment

oliviertassinari commented Sep 16, 2019

eps1lon commented Sep 17, 2019

oliviertassinari commented Sep 17, 2019 • edited

eps1lon commented Sep 17, 2019

eps1lon commented Sep 18, 2019

oliviertassinari commented Sep 18, 2019 • edited

eps1lon commented Sep 18, 2019

oliviertassinari commented Sep 18, 2019

jkeruzec commented Sep 23, 2019

oliviertassinari commented Sep 23, 2019 • edited

croraf commented Sep 15, 2019 •

edited

oliviertassinari commented Sep 17, 2019 •

edited

oliviertassinari commented Sep 18, 2019 •

edited

oliviertassinari commented Sep 23, 2019 •

edited