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

IE 11 and Edge no longer prompt to remember password on controlled form #7328

Open
lPadier opened this issue Jul 21, 2016 · 14 comments · Fixed by #11534
Open

IE 11 and Edge no longer prompt to remember password on controlled form #7328

lPadier opened this issue Jul 21, 2016 · 14 comments · Fixed by #11534

Comments

@lPadier
Copy link

lPadier commented Jul 21, 2016

Do you want to request a feature or report a bug?
Bug (regression) IE/Edge

What is the current behavior?
On React > v15.2.0, Edge and IE11 do not prompt the user to save the password for the form

Steps to reproduce
With react 15.1.0:
https://jsfiddle.net/69z2wepo/49876/
With React 15.2.0:
https://jsfiddle.net/69z2wepo/49877/

What is the expected behavior?
The browser prompts the user to save their password for the form

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React > 15.2.0. It worked in React 15.2.0
IE 11, Edge

@jdalton
Copy link
Contributor

jdalton commented Nov 10, 2016

This issue is caused by this commit 4338c8d#diff-5a2abece6f8c2495d8242daa40245434L156
which changed:

value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS,

to

value: 0,

It looks like the key is the HAS_SIDE_EFFECTS bit.
Changing the value, even if it's identical to the existing value, knocks off browser helpers in Edge.

I see that HAS_SIDE_EFFECTS was removed in a later version so that's something to account for.

Related to #6406 \cc @jimfb.

@jdalton
Copy link
Contributor

jdalton commented Nov 18, 2016

@lPadier I've opened a PR at #8266.

@florian-crewmeister
Copy link

Any progress on this bug? The PR to fix it seems to be hanging around for a while and has not yet been merged.

@nhunzaker
Copy link
Contributor

@aweary @jquense We should get test fixture coverage on this. It would be our first browser specific test case!

@nippur72
Copy link

is there any workaround for this issue?

@gaearon gaearon changed the title Regression React >=15.2.0: IE 11 and Edge no longer prompt to remember password on controlled form IE 11 and Edge no longer prompt to remember password on controlled form Oct 4, 2017
nhunzaker added a commit that referenced this issue Nov 12, 2017
This commit replaces the method of synchronizing an input's value
attribute from using setAttribute to assigning defaultValue. This has
several benefits:

- Fixes issue where IE10+ and Edge password icon disappears (#7328)
- Fixes issue where toggling input types hides display value on dates
  in Safari (unreported)
- Removes mutationMethod behaviors from DOMPropertyOperations
nhunzaker added a commit that referenced this issue Nov 12, 2017
This commit replaces the method of synchronizing an input's value
attribute from using setAttribute to assigning defaultValue. This has
several benefits:

- Fixes issue where IE10+ and Edge password icon disappears (#7328)
- Fixes issue where toggling input types hides display value on dates
  in Safari (unreported)
- Removes mutationMethod behaviors from DOMPropertyOperations
nhunzaker added a commit that referenced this issue Nov 27, 2017
This commit replaces the method of synchronizing an input's value
attribute from using setAttribute to assigning defaultValue. This has
several benefits:

- Fixes issue where IE10+ and Edge password icon disappears (#7328)
- Fixes issue where toggling input types hides display value on dates
  in Safari (unreported)
- Removes mutationMethod behaviors from DOMPropertyOperations
nhunzaker added a commit that referenced this issue Nov 29, 2017
This commit replaces the method of synchronizing an input's value
attribute from using setAttribute to assigning defaultValue. This has
several benefits:

- Fixes issue where IE10+ and Edge password icon disappears (#7328)
- Fixes issue where toggling input types hides display value on dates
  in Safari (unreported)
- Removes mutationMethod behaviors from DOMPropertyOperations
gaearon pushed a commit that referenced this issue Nov 30, 2017
* Use defaultValue instead of setAttribute('value')

This commit replaces the method of synchronizing an input's value
attribute from using setAttribute to assigning defaultValue. This has
several benefits:

- Fixes issue where IE10+ and Edge password icon disappears (#7328)
- Fixes issue where toggling input types hides display value on dates
  in Safari (unreported)
- Removes mutationMethod behaviors from DOMPropertyOperations

* initialValue in Input wrapperState is always a string

* The value property is assigned before the value attribute. Fix related tests.

* Remove initial value tests in ReactDOMInput

I added these tests after removing the `value` mutation
method. However they do not add any additional value over existing
tests.

* Improve clarity of value checks in ReactDOMInput.postMountWrapper

* Remove value and defaultValue from InputWithWrapperState type

They are already included in the type definition for HTMLInputElement

* Inline stringification of value in ReactDOMInput

Avoids eagier stringification and makes usage more consistent.

* Use consistent value/defaultValue presence in postMountHook

Other methods in ReactDOMInput check for null instead of
hasOwnProperty.

* Add missing semicolon

* Remove unused value argument in ReactDOMInput test

* Address cases where a value switches to undefined

When a controlled input value switches to undefined, it reverts back
to the initial state of the controlled input.

We didn't have test coverage for this case, so I've added two describe
blocks to cover both null and undefined.
@PaloSP
Copy link

PaloSP commented Mar 27, 2018

For me this bug is still present in 16.2.0.
IE: 11.309.16299.0
EDGE : 41.16299.248.0

@luisrudge
Copy link

I'm still having this issue with both IE and Edge:

Works with 15.1.0: https://codesandbox.io/s/lpvz0zy9wq
Doesn't work with 15.2.0: https://codesandbox.io/s/o9kl2jzo1q
Doesn't work with 15.6.2: https://codesandbox.io/s/38kp95wl96
Doesn't work with 16.3.1: https://codesandbox.io/s/xjpk3wr55p

In order to test this effectively, you have to follow a few steps:

Using Edge, go to Settings >> View Advanced Settings, under Privacy and Services

  • make sure you have Offer to save passwords enabled:
    image

  • make sure you don't have any passwords saved in the codesandbox.io domain:
    image

Steps to reproduce when IT WORKS (15.1.0)

  • go to the 15.1.0 sandbox
  • type a username and a password and hit submit
  • you'll see a confirmation panel about saving passwords:
    image
  • refresh the page
  • type another username and a password and hit submit
  • you'll see another confirmation panel about saving passwords:
    image
  • refresh the page
  • click on the first input
  • you'll see a list of the previously used usernames: image

Steps to reproduce when IT DOESN'T WORK (15.2.0, 15.6.2, 16.3.1)

  • remove previously saved passwords from the codesandbox.io domain:
    image
  • go to any of the non-working sandboxes (15.2.0, 15.6.2, 16.3.1
  • type a username and a password and hit submit
  • there will be no password confirmation panel
  • refresh the page
  • click on the first input
  • there will be no list of previously used usernames

@abbybhat
Copy link

Is there a work around for this issue? I tried updating react to 16.4.1, didnt fix the issue for me

@nhunzaker
Copy link
Contributor

@abbybhat Oh no! Can you send over a reproduction case?

@danielpater
Copy link

@nhunzaker The reproduction case was already provided by @luisrudge above. I updated it to 16.4.1:

Password prompt in IE 11 showing with 15.2.0:
https://lpvz0zy9wq.codesandbox.io/

Password prompt in IE 11 not showing with 16.4.1:
https://q7knzjrk8q.codesandbox.io/

@gaearon gaearon reopened this Aug 29, 2018
@gaearon
Copy link
Collaborator

gaearon commented Aug 29, 2018

Seems like this wasn't fixed after all. We need to figure out why we thought this was fixed in #11534.

@nhunzaker
Copy link
Contributor

There might have been a mismatch with the originally referenced issue in #8266, which was focused on the password reveal icon.

@nhunzaker
Copy link
Contributor

I believe this works in Edge, but not IE11. I am still confirming IE11, but I need to step away for a bit. Here is what I found:

I setup a fixture for this, and I'm not having an issue getting the "remember password" flow". I have made a test fixture for this case here:

http://react-password-remember.surge.sh/password-inputs

And the associated diff:
https://github.com/facebook/react/compare/master...nhunzaker:password-remember-test-fixture?expand=1

screen shot 2018-08-29 at 4 32 16 pm

This holds true for master, React 16.4, and React 16.3.

Additionally, I am not having an issue triggering the password flow in @luisrudge's example:
screen shot 2018-08-29 at 4 36 14 pm

And I can also see the passwords saved in the manager:

screen shot 2018-08-29 at 4 39 26 pm

@nhunzaker
Copy link
Contributor

Looks like we already came to this conclusion in #12749. This does not work in IE11, and was being tracked to that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.