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

st.form() not updating values on password auto-full (regression in Streamlit 1.25) #7101

Closed
4 tasks done
xl0 opened this issue Aug 1, 2023 · 11 comments
Closed
4 tasks done
Assignees
Labels
feature:st.form priority:P2 status:awaiting-user-response Issue requires clarification from submitter status:confirmed Bug has been confirmed by the Streamlit team type:bug Something isn't working

Comments

@xl0
Copy link

xl0 commented Aug 1, 2023

Checklist

  • I have searched the existing issues for similar issues.
  • I added a very descriptive title to this issue.
  • I have provided sufficient information below to help reproduce this issue.

Summary

1.25.0 partially ignores form auto-fill.

Let's say, I have a login form with username and password fields.
The password is saved by the browser. I click on the password field and select the saved password.
Both the username and password fields are auto-filled by the browser:

image

The form should return the username and password, but in 1.25.0 only the password is being set.

In 1.24.1:
user=user
password=sdfsfdsfdsfsd

In 1.25.0:
user=
password=sdfsfdsfdsfsd

The username field returns an empty string, despite being auto-filled by the browser.

Reproducible Code Example

import streamlit as st


if "logged_in" not in st.session_state:
    form = st.form(key="my_form")

    form.subheader("Login")

    user = form.text_input("Username")
    password = form.text_input("Password", type="password", autocomplete="new-password")

    if form.form_submit_button("Login"):
        # Check if username and password are correct

        st.session_state["user"] = user
        st.session_state["password"] = password
        st.session_state["logged_in"] = True
        st.experimental_rerun()


else:
    st.subheader("Logged in")
    st.write("You are logged in.")

    st.write("username:", st.session_state["user"])
    st.write("password:", st.session_state["password"])

    if st.button("Logout"):
        del st.session_state["logged_in"]
        st.experimental_rerun()

Steps To Reproduce

No response

Expected Behavior

No response

Current Behavior

No response

Is this a regression?

  • Yes, this used to work in a previous version.

Debug info

  • Streamlit version: 1.25.0
  • Python version: 3.11.4
  • Operating System: Ubuntu 22.04
  • Browser: Goole Chrome Version 115.0.5790.102 (Official Build) (64-bit)

Additional Information

No response

@xl0 xl0 added status:needs-triage Has not been triaged by the Streamlit team type:bug Something isn't working labels Aug 1, 2023
@xl0
Copy link
Author

xl0 commented Aug 1, 2023

This could be related to #7084

@kmcgrady
Copy link
Collaborator

kmcgrady commented Aug 2, 2023

Hey @xl0. This issue has me stumped. We made a change on pressing Enter on a text input, to submit a form, but this seems different. Couple questions:

  1. When you fill in the password, is the form being auto submitted (I wonder if Enter is being sent too soon before the username gets set.
  2. Regardless of the difference in versions. For a login, shouldn't the username have a autocomplete="username"? That seems like the preferred setup.
  3. Is this true across all browsers?

Thanks for your help!

@kmcgrady kmcgrady added status:awaiting-user-response Issue requires clarification from submitter and removed status:needs-triage Has not been triaged by the Streamlit team labels Aug 2, 2023
@xl0
Copy link
Author

xl0 commented Aug 3, 2023

@kmcgrady

  1. No, I still have to push the submit button
  2. No, I think streamlit sets autocompelte= automatically. I did not even have to set autocomplete="new-password" for it to work. I only set it as part of debugging the issues. It should not have been part of the example I gave.
  3. On firefox the behaviour is different (but still wrong).
    If I click on the password field, the user field does not get auto-filled and is just left blank.
    If I click on the user field, both user and password are auto-filled, but the form returns

user=user
password=

This might be a separate bug.

I also noticed, in Chrome, if I interact with the Username field at all, it does get populated. See the gif:

Peek 2023-08-03 12-47

@carolinefrasca
Copy link
Contributor

Seems like this forum thread is related

@willhuang1997
Copy link
Contributor

Hi, I will be investigating this later this week. I am currently working on another bug. I definitely find this bug to be super annoying for everyone so hopefully i'll root cause it soon and fix it soon.

@willhuang1997
Copy link
Contributor

willhuang1997 commented Aug 8, 2023

I was able to reproduce on this streamlit cloud app.
https://willhuang1997-st-cloud-testing-debug-njmh2w.streamlit.app/

I think this is probably a bug created by me and hopefully I'll have a fix soon.

@willhuang1997 willhuang1997 added priority:P2 status:confirmed Bug has been confirmed by the Streamlit team labels Aug 8, 2023
sfc-gh-wihuang pushed a commit that referenced this issue Aug 12, 2023
#7150)

Describe your changes
Change commitWidgetValue to take in dirty parameter as we need to change dirty to true when committing the widget value
we need to commit the widget value because when autofill is done, the widget value is not committed as thus username will show up to be an empty string without interaction
GitHub Issue Link (if applicable)
#7101
#7084

Testing Plan
Explanation of why no additional tests are needed
Unit Tests (JS and/or Python)
readded tests I deleted before with some slight additions
E2E Tests
Any manual testing needed?
yes
it doesn't seem very doable to test autofill functionality. However, I can probably add some tests for commit value being called
Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
@sfc-gh-dmatthews
Copy link
Collaborator

@willhuang1997 Does #7150 close this issue?

@willhuang1997
Copy link
Contributor

@willhuang1997 Does #7150 close this issue?

Yep

@ctrimborn
Copy link

ctrimborn commented Sep 6, 2023

@willhuang1997 Thank you for your work. Apparently, your bugfix has been added to the latest version of Streamlit. Unfortunately, the error still appears in the following code. While using Firefox and Chrome autofill

import streamlit as st



email = str(st.text_input("Email", placeholder="name@email.de")).lower()
password = str(st.text_input("Password", type='password', placeholder="********"))
if st.button("Submit"):
    st.write(email)
    st.write(password)

@willhuang1997
Copy link
Contributor

@ctrimborn , You're using st.button and not st.submit_button. I think this issue is specifically for st.form_button(). However, you're using st.button which is different. Can you create a separate bug for this?

@nicedouble
Copy link

+1 for this issue,both st.form_submit_button and st.button.

eric-skydio pushed a commit to eric-skydio/streamlit that referenced this issue Dec 20, 2023
streamlit#7150)

Describe your changes
Change commitWidgetValue to take in dirty parameter as we need to change dirty to true when committing the widget value
we need to commit the widget value because when autofill is done, the widget value is not committed as thus username will show up to be an empty string without interaction
GitHub Issue Link (if applicable)
streamlit#7101
streamlit#7084

Testing Plan
Explanation of why no additional tests are needed
Unit Tests (JS and/or Python)
readded tests I deleted before with some slight additions
E2E Tests
Any manual testing needed?
yes
it doesn't seem very doable to test autofill functionality. However, I can probably add some tests for commit value being called
Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
zyxue pushed a commit to zyxue/streamlit that referenced this issue Mar 22, 2024
streamlit#7150)

Describe your changes
Change commitWidgetValue to take in dirty parameter as we need to change dirty to true when committing the widget value
we need to commit the widget value because when autofill is done, the widget value is not committed as thus username will show up to be an empty string without interaction
GitHub Issue Link (if applicable)
streamlit#7101
streamlit#7084

Testing Plan
Explanation of why no additional tests are needed
Unit Tests (JS and/or Python)
readded tests I deleted before with some slight additions
E2E Tests
Any manual testing needed?
yes
it doesn't seem very doable to test autofill functionality. However, I can probably add some tests for commit value being called
Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
zyxue pushed a commit to zyxue/streamlit that referenced this issue Apr 16, 2024
streamlit#7150)

Describe your changes
Change commitWidgetValue to take in dirty parameter as we need to change dirty to true when committing the widget value
we need to commit the widget value because when autofill is done, the widget value is not committed as thus username will show up to be an empty string without interaction
GitHub Issue Link (if applicable)
streamlit#7101
streamlit#7084

Testing Plan
Explanation of why no additional tests are needed
Unit Tests (JS and/or Python)
readded tests I deleted before with some slight additions
E2E Tests
Any manual testing needed?
yes
it doesn't seem very doable to test autofill functionality. However, I can probably add some tests for commit value being called
Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:st.form priority:P2 status:awaiting-user-response Issue requires clarification from submitter status:confirmed Bug has been confirmed by the Streamlit team type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants