Android - Attachments - Typing is not smooth when revealing password in protected PDF
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.69-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5284678&group_by=cases:section_id&group_order=asc&group_id=292107 Email or phone of affected tester (no customers): [email protected]* Issue reported by: Applause - Internal Team
Action Performed:
- Open the Expensify app
- Open any chat
- Tap on the "+" button and select "Add File"
- Select a PDF that is protected with a password
- Tap on "Enter the password"
- Tap on the eye icon to reveal the password
- Start typing the password in a fast manner
- Verify that each character is added smoothly
Expected Result:
When typing the PDF password after revealing it, each character should be added smotthly
Actual Result:
Typing is not smooth when typing PDF password after revealing it. When typing in a fast manner, not every character is added to password
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Standalone
- [x] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/e3809f66-d637-454a-a280-74b66f08b6bc
Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!
I couldn't action this today. I will check it out this weekend.
Job added to Upwork: https://www.upwork.com/jobs/~021866173601902671628
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Typing is not smooth when revealing password in protected PDF. On Android, the keyboard flickers when typing the password in the revealed state.
What is the root cause of that problem?
React Native’s TextInput automatically switches the keyboard type on Android based on the secureTextEntry prop. See this comment that shows the behavior for both IOS and Android when keyboardType=numeric.
We are using the getSecureEntryKeyboardType method to determine the keyboard here:
https://github.com/Expensify/App/blob/f481d64b7e894b6a5a98dcaec0e48876a42e60f9/src/components/TextInput/BaseTextInput/index.native.tsx#L370
This is the method responsible for determining keyboard type on Android devices:-
The keyboardType='visible-password' conflicts with the default behavior of switching keyboard type on Android. It results in the flickering keyboard that can be seen in this screencast below:-
Observe the
emojiicon button and themic button.
https://github.com/user-attachments/assets/4b24a7bd-9e3a-4c91-abc2-937e0e53ea01
What changes do you think we should make in order to solve the problem?
We should not use the native getSecureEntryKeyboardType method to determine keyboardType based on passwordHidden and secureTextEntry parameters on Android devices, instead we should use directly assign
keyboardType={inputProps.keyboardType}
It fixes the issue as seen in the screencast
https://github.com/user-attachments/assets/539d31ed-add4-4c6a-a771-109255cc0b7d
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
-
We can add tests, if not already, for the
native getSecureEntryKeyboardTypemethod ensuring that it returns the passedkeyboardTypeprop as it is, if prefer refactoring similar to the method in src/libs/getSecureEntryKeyboardType/index.ts file. -
Otherwise, just get rid of that
index.android.tsfile and write tests, if not already, for the method in src/libs/getSecureEntryKeyboardType/index.ts.
What alternative solutions did you explore? (Optional)
-
Alternatively, we can also pass the
inputMethodprop astextto the TextInput component in PDFPasswordForm.tsx Because the inputMethod prop has precedence over the keyboardType prop and hence will ignore thekeyboardType=visible-password. See this ref -
However, I would suggest removing the usage of the native
getSecureEntryKeyboardTypemethod because this is getting called on each keystroke(verified by log statements). We already have the same method that directly returns the passedkeyboardTypeprop which is being used for Web/Desktop/IOS/.
@rohit9625 Thanks for the proposal! What version of Android are you able to reproduce the issue on? For me on Android 14 (API 34) I don't see the issue.
Your solution would introduce a regression from the fix implemented in https://github.com/Expensify/App/pull/9593, which was added to prevent the keyboard from visibly switching when toggling the password visibility (which you can see happens in your solution video).
What version of Android are you able to reproduce the issue on? For me on Android 14 (API 34) I don't see the issue.
I tested this issue on my Samsung A14 Device with Android 14(API 34).
Your solution would introduce a regression from the fix implemented in #9593, which was added to prevent the keyboard from visibly switching when toggling the password visibility (which you can see happens in your solution video).
I'm looking into it. How can I access that password input screen while login? For now, I'm being redirected to the Magic Code screen.
@rohit9625 Huh, strange that I can't reproduce it on the same API version. Does it happen for you in an emulator too?
How can I access that password input screen while login?
That no longer exists in the app, but the behaviour is the same in the PDF password input since it's the same base component.
@jjcoffee, I haven't tried it on an emulator yet. I'll try and let you know.
I don't know why but the build failed when I pulled from the main branch recently. Do you have any idea about this failing task? I was building the app directly using the command ./gradlew buildDevelopmentDebug.
Task :react-native-key-command:generateCodegenSchemaFromJavaScript No modules to process in combine-js-to-schema-cli. If this is unexpected, please check if you set up your NativeComponent correctly. See combine-js-to-schema.js for how codegen finds modules.
Task :react-native:packages:react-native:ReactAndroid:hermes-engine:downloadHermes Download https://github.com/facebook/hermes/tarball/hermes-2024-08-15-RNv0.75.1-4b3bf912cc0f705b51b71ce1a5b8bd79b93a451b
Task :app:generateAutolinkingPackageList FAILED
Task :react-native:packages:react-native:ReactAndroid:downloadBoost UP-TO-DATE Download https://archives.boost.io/release/1.83.0/source/boost_1_83_0.tar.gz
Task :react-native:packages:react-native:ReactAndroid:downloadDoubleConversion UP-TO-DATE Download https://github.com/google/double-conversion/archive/v1.1.6.tar.gz
Task :react-native:packages:react-native:ReactAndroid:hermes-engine:downloadHermes UP-TO-DATE
Task :react-native:packages:react-native:ReactAndroid:downloadFmt UP-TO-DATE Download https://github.com/fmtlib/fmt/archive/9.1.0.tar.gz
Task :react-native:packages:react-native:ReactAndroid:downloadFolly UP-TO-DATE Download https://github.com/facebook/folly/archive/v2024.01.01.00.tar.gz
Task :react-native:packages:react-native:ReactAndroid:downloadGlog UP-TO-DATE Download https://github.com/google/glog/archive/v0.3.5.tar.gz
Task :react-native:packages:react-native:ReactAndroid:downloadGtest UP-TO-DATE Download https://github.com/google/googletest/archive/refs/tags/release-1.12.1.tar.gz
FAILURE: Build failed with an exception.
- What went wrong: Execution failed for task ':app:generateAutolinkingPackageList'.
RNGP - Autolinking: Could not find project.android.packageName in react-native config output! Could not autolink packages without this field.
- Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights. Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
For more on this, please refer to https://docs.gradle.org/8.8/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.
BUILD FAILED in 1m 3s 262 actionable tasks: 157 executed, 105 up-to-date
Hey @jjcoffee, I tried running the app on the emulator as well with API 34 and still facing the same issue. See the screencast below:-
https://github.com/user-attachments/assets/3ff2878b-c7e6-4942-9b68-56cbe5843190
I think that toggling secureTextEntry automatically switches the keyboardType as I mentioned in my proposal. Currently, we are forcing keyboardType='visible-password' when the password is not hidden which is redundant. Please correct me if I'm wrong.
https://github.com/Expensify/App/blob/20b4c7db47969f7d2209f1c0b8fd765ce883f92c/src/libs/getSecureEntryKeyboardType/index.android.ts#L8-L9
I guess the behavior in my solution video is obvious. The same behavior can be seen for Facebook Android app below:-
https://github.com/user-attachments/assets/0b912d33-1db0-4ad6-ad01-eba790916260
This is the screencast from the emulator after the fix I mentioned. The behavior in the screencast was the issue that this #9593 has solved.
https://github.com/user-attachments/assets/520a4eed-430c-41a9-b09e-47a43de55702
I think we cannot prevent this keyboard switching because this is the library's issue. However, when secureTextEntry={true} the keyboard prevents autoCorrect and autoComplete but it allows that when secureTextEntry={false}.
Thanks for the extra testing @rohit9625! I think I'm not quite convinced by the RCA in your proposal, as the behaviour is that the keyboard is switching whilst typing, which doesn't seem to be explained by setting keyboardType='visible-password' (unless this is some sort of known bug).
@johncschuster Do you think it's an acceptable fix if the keyboard changes once you switch between visible/invisible modes? It's technically a regression from https://github.com/Expensify/App/pull/9593, but it's visually less disturbing than the keyboard changing whilst you type, I think.
@johncschuster @jjcoffee this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@johncschuster, @jjcoffee Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@johncschuster Friendly bump on this question :pray:
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Edited by proposal-police: This proposal was edited at 2024-12-18 16:28:23 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Android - Attachments - Typing is not smooth when revealing password in a protected PDF
What is the root cause of that problem?
When the password is visible, we set secureTextEntry to false and keyboardType to visible-password.
When the password is hidden, we set secureTextEntry to true and keyboardType to undefined.
https://github.com/Expensify/App/blob/4b0f250c71d4cc214107ff84797b24365964a5d0/src/components/TextInput/BaseTextInput/index.native.tsx#L367-L370
The issue lies in the native setSecureTextEntry method here
@ReactProp(name = "secureTextEntry", defaultBoolean = false)
public void setSecureTextEntry(ReactEditText view, boolean password) {
updateStagedInputTypeFlag(
view,
password
? InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD
: InputType.TYPE_NUMBER_VARIATION_PASSWORD | InputType.TYPE_TEXT_VARIATION_PASSWORD,
password ? InputType.TYPE_TEXT_VARIATION_PASSWORD : 0);
checkPasswordType(view);
}
When secureTextEntry is set to true, it sets the input type to TYPE_TEXT_VARIATION_PASSWORD flag and removes TYPE_TEXT_VARIATION_VISIBLE_PASSWORD.
When secureTextEntry is set to false, it does not set a new input type to anything but removes TYPE_NUMBER_VARIATION_PASSWORD and TYPE_TEXT_VARIATION_PASSWORD. Because it didn't set a new type, it defaults to the default (which is not visible-password that we expect).
Note: On every render the order in which these methods are called is reversed (I think it's by design to avoid relying on the order):
setSecureTextEntry(false)
setKeyboardType(visible-password)
setKeyboardType(visible-password)
setSecureTextEntry(false)
setSecureTextEntry(false)
setKeyboardType(visible-password)
setKeyboardType(visible-password)
setSecureTextEntry(false)
When setSecureTextEntry(false) is called second, it removes TYPE_TEXT_VARIATION_PASSWORD (as described above) that was added by setKeyboardType here.
This causes the keyboard to switch to a different one on every keystroke: visible-password -> default -> visible-password -> default.
What changes do you think we should make in order to solve the problem?
Patch this line to set the input type back to TYPE_TEXT_VARIATION_VISIBLE_PASSWORD, and possibly make an upstream PR
@ReactProp(name = "secureTextEntry", defaultBoolean = false)
public void setSecureTextEntry(ReactEditText view, boolean password) {
updateStagedInputTypeFlag(
view,
password
? InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD
: InputType.TYPE_NUMBER_VARIATION_PASSWORD | InputType.TYPE_TEXT_VARIATION_PASSWORD,
- password ? InputType.TYPE_TEXT_VARIATION_PASSWORD : 0);
+ password
+ ? InputType.TYPE_TEXT_VARIATION_PASSWORD
+ : (view.getStagedInputType() & InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) == InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD
+ ? InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD
+ : 0);
checkPasswordType(view);
}
Note: this is not thoroughly tested so it would be good to hear the RN team's opinion first.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
Our tests won't be able to catch this as they only run against the web platform and this is an Android issue.
Thanks for the extra testing @rohit9625! I think I'm not quite convinced by the RCA in your https://github.com/Expensify/App/issues/53394#issuecomment-2532202683, as the behaviour is that the keyboard is switching whilst typing, which doesn't seem to be explained by setting keyboardType='visible-password' (unless this is some sort of known bug).
@johncschuster Do you think it's an acceptable fix if the keyboard changes once you switch between visible/invisible modes? It's technically a regression from https://github.com/Expensify/App/pull/9593, but it's visually less disturbing than the keyboard changing whilst you type, I think.
@jjcoffee sorry I missed your message above! I'm generally not a fan of regressing, but before I weigh in, I think I need a bit more clarity to make sure I'm following along.
Can you help me understand how the keyboard would change once we switch between visible/invisible modes? Is it a visual change, or is this simply switching the keyboard to secureTextEntry={true}, which prevents autoCorrect and autoComplete?
Hey @johncschuster, please refer to this comment to see the visual changes in the keyboard on Emulator, before and after the fix.
The keyboard might be flickering because we simultaneously have secureTextEntry={true} and keyboardType="visible-password" in our code.
But, setting keyboardType="visible-password" supersedes that secureTextEntry={false}, according to the library here:-
@ReactProp(name = "keyboardType")
public void setKeyboardType(ReactEditText view, @Nullable String keyboardType) {
int flagsToSet = InputType.TYPE_CLASS_TEXT;
if (KEYBOARD_TYPE_NUMERIC.equalsIgnoreCase(keyboardType)) {
flagsToSet = INPUT_TYPE_KEYBOARD_NUMBERED;
} else if (KEYBOARD_TYPE_NUMBER_PAD.equalsIgnoreCase(keyboardType)) {
flagsToSet = INPUT_TYPE_KEYBOARD_NUMBER_PAD;
} else if (KEYBOARD_TYPE_DECIMAL_PAD.equalsIgnoreCase(keyboardType)) {
flagsToSet = INPUT_TYPE_KEYBOARD_DECIMAL_PAD;
} else if (KEYBOARD_TYPE_EMAIL_ADDRESS.equalsIgnoreCase(keyboardType)) {
flagsToSet = InputType.TYPE_TEXT_VARIATION_EMAIL_ADDRESS | InputType.TYPE_CLASS_TEXT;
// Set cursor's visibility to False to fix a crash on some Xiaomi devices with Android Q. This
// crash happens when focusing on a email EditText, during which a prompt will be triggered
// but
// the system fail to locate it properly. Here is an example post discussing about this
// issue: https://github.com/facebook/react-native/issues/27204
if (shouldHideCursorForEmailTextInput()) {
view.setCursorVisible(false);
}
} else if (KEYBOARD_TYPE_PHONE_PAD.equalsIgnoreCase(keyboardType)) {
flagsToSet = InputType.TYPE_CLASS_PHONE;
} else if (KEYBOARD_TYPE_VISIBLE_PASSWORD.equalsIgnoreCase(keyboardType)) {
// This will supersede secureTextEntry={false}. If it doesn't, due to the way
// the flags work out, the underlying field will end up a URI-type field.
flagsToSet = InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD;
} else if (KEYBOARD_TYPE_URI.equalsIgnoreCase(keyboardType)) {
flagsToSet = InputType.TYPE_TEXT_VARIATION_URI;
}
updateStagedInputTypeFlag(view, InputType.TYPE_MASK_CLASS, flagsToSet);
checkPasswordType(view);
}
Hence, results in conflict:
secureTextEntry=false by setting keyboardType="visible-password" and secureTextEntry=true by passing this value as true from our App's code.
But, setting keyboardType="visible-password" supersedes that secureTextEntry={false}, according to the library here:-
This is true, but it depends on the order in which these values are set. It will only supersede if setSecureTextEntry was called before setKeyboardType, not the other way around. My proposal suggests a fix to setSecureTextEntry to account for cases when it is called after setKeyboardType.
Can you help me understand how the keyboard would change once we switch between visible/invisible modes? Is it a visual change, or is this simply switching the keyboard to secureTextEntry={true}, which prevents autoCorrect and autoComplete?
@johncschuster Just to add in text form, yes this is purely a visual change. Namely the emoji picker on the keyboard becomes visible as well as the number bar at the top of the keyboard being replaced by the auto-correct/complete suggestions.
@CyberAndrii Thanks for your proposal! Normally I think fixing upstream would be the best way forward, but I'm unsure if this is something they'd actually want to support (since us setting the keyboardType directly is a bit of a workaround anyway).
I think it makes sense to go forwards with @rohit9625's proposal, with the acceptance that the keyboard will switch when tapping the visibility icon (this is normal behaviour in other apps as pointed out here). Open to discussion though, if the assigned engineer disagrees!
:ribbon::eyes::ribbon: C+ reviewed
Triggered auto assignment to @rafecolton, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
I'm not sure that When user tapped on the eye icon in password field on Android, the keyboard changes was actually a bug, it sounds like a feature to me. As a user, I see this kind of thing when I tap into an email field, for example, so I think it's a pattern that will be understood by users. A flickering or unresponsive keyboard seems like a much bigger problem.
@tgolen I'm curious what you think, since you reviewed https://github.com/Expensify/App/pull/9593.
I think the keyboard switching sounds fine.
@rafecolton, @johncschuster, @jjcoffee Whoops! This issue is 2 days overdue. Let's get this updated quick!
Not overdue, @rafecolton is OOO, so we'll get back to this next year I guess :partying_face:
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@rafecolton, @johncschuster, @jjcoffee Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!