-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: remove double <form> element from 2FA form #9851
base: main
Are you sure you want to change the base?
Conversation
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build failed: ✖️ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9851 +/- ##
============================================
- Coverage 15.78% 15.78% -0.01%
- Complexity 12564 12565 +1
============================================
Files 5627 5627
Lines 492250 492250
Branches 61405 63543 +2138
============================================
- Hits 77710 77708 -2
- Misses 406066 406067 +1
- Partials 8474 8475 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build failed: ✖️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm, I cannot see a functional difference in your video snippets (before/after) @phsm?
Thanks for pointing that out. I indeed made it unclear. Replaced the first video. |
Hello, |
@phsm , the rule is two reviews of which at least one has done testing. So far I am the only one that has reviewed. You could ask any active committer to review as well. A good trick is usually to look at the git blame output to find someone with experience/knowledge of the involved code. Just keep in mind that some may not be active anymore. |
@rohityadavcloud I see you have made changes to this file in the past. Perhaps you can take a look? |
Description
This PR fixes the password managers autofill functionality in the 2FA OTP code form.
It is caused by double
<form>
element.Fixes: #9510
The PR simply removes the outer empty
<form>
element, leaving the inner one intact.Steps to reproduce:
Expected behavior:
1password fills all 6 numbers of the OTP token into the input form.
Actual behavior:
1password only fills 3 numbers out of 6 into the input form.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Before:
2fa_current.mp4
After:
2fa_fixed.mp4
How Has This Been Tested?
How did you try to break this feature and the system with this change?