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.

Sign up for GitHub

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

Jump to bottom

bug: IonInput type="password" clears input field on modifier key press #28633

Closed
3 tasks done
chawes13 opened this issue Dec 5, 2023 · 5 comments · Fixed by #28639
Closed
3 tasks done

bug: IonInput type="password" clears input field on modifier key press #28633

chawes13 opened this issue Dec 5, 2023 · 5 comments · Fixed by #28639
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@chawes13
Copy link

chawes13 commented Dec 5, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

When IonInput is given a value of type="password" in @ionic/react, the input will clear text unexpectedly when certain keys are pressed. This is most notable when using a keyboard to navigate between controls. This behavior is not observed with the native input element with type set to password or the default IonInput.

Expected Behavior

IonInputs should be accessible via keyboard. Pressing modifier keys (e.g., shift, command) should not clear a password input (unless combined with another key, e.g., shift + p).

Steps to Reproduce

  1. Create an IonInput with type="password"
  2. Enter a value (either via keyboard or via password manager autofill)
  3. Tab to the next focusable element
  4. Tab back to the password input
  5. Tab to the next focusable element
  6. IonInput with type="password" is empty

Code Reproduction URL

https://stackblitz.com/edit/vxete3-yk4b3w?file=src%2Fmain.tsx

Ionic Info

Via StackBlitz:

Ionic:

Ionic CLI : 5.4.16

Utility:

cordova-res : not installed
native-run : not installed

System:

NodeJS : v18.18.0
npm : 9.4.2
OS : Linux 5.0

Additional Information

First noticed on a project using @ionic/react: 7.0.4 and validated on latest available release in StackBlitz (i.e., "@ionic/react": "^7.5.7" and "@ionic/react-router": "^7.5.7").
@ionitron-bot ionitron-bot bot added the triage label Dec 5, 2023
@liamdebeasi liamdebeasi self-assigned this Dec 5, 2023
@liamdebeasi
Copy link
Contributor

Thanks for the report. We had a partial fix in Ionic 7.3.1 which accounts for going from a password input to a non-password input, but we forgot to account for the Shift key modifier. I'll work on a fix for this.

@liamdebeasi
Copy link
Contributor

Actually it looks like we do not account for several keys including "Control", "Meta", and "Shift". The problem here is the Shift key gets dispatched first when tabbing backwards. Since Shift is not explicitly excluded, the input gets cleared.

Worth noting that your demo was using Ionic 7.0.10 (the package-lock.json had that specified), so you did not have the 7.3.1 fix. However, there are other edge cases we need to fix too.

@chawes13
Copy link
Author

chawes13 commented Dec 5, 2023

@liamdebeasi huh, weird. Guess I need to file a Stackblitz issue now 😅 . Sorry about that!

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Dec 5, 2023
@ionitron-bot ionitron-bot bot removed triage labels Dec 5, 2023
@liamdebeasi liamdebeasi mentioned this issue Dec 5, 2023
Merged
2 tasks
@liamdebeasi liamdebeasi removed their assignment Dec 5, 2023
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Dec 5, 2023 •
edited

Here's a dev build if you are interested in testing the proposed fix: 7.5.8-dev.11701791090.18ebbea8

Install Example:

npm install @ionic/react@7.5.8-dev.11701791090.18ebbea8 @ionic/react-router@7.5.8-dev.11701791090.18ebbea8

github-merge-queue bot pushed a commit that referenced this issue Dec 11, 2023
@liamdebeasi
fix(input, textarea): clearOnInput ignores key modifiers (#28639)
8f7d87c 
Issue number: resolves #28633

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

In #28005 I introduced
a fix that causes "clearOnEdit" to not clear input/textarea when the Tab
key was pressed. However, there were several edge cases I missed. For
instance, pressing the "shift" key and then the "tab" key would still
clear the input because "shift" was not explicitly excluded.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Input and textarea now explicitly ignores modifier keys that do not
change the value of the input
- `didInputClearOnEdit` is not set to `true` when an ignored key is
pressed. Otherwise, pressing an ignored key and then an accepted key
would not cause the input to be cleared. For example, pressing "shift",
releasing "shift", then pressing "A" should cause the input to still get
cleared.
- Added test coverage

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Playwright bug report for
a9f34a5:
microsoft/playwright#28495
@ionitron-bot Ionitron Bot
Copy link

ionitron-bot bot commented Jan 10, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jan 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(input, textarea): clearOnInput ignores key modifiers ionic-team/ionic-framework
2 participants
@liamdebeasi @chawes13