bpellin / keepassdroid

KeePass implementation for android
http://www.keepassdroid.com
Other
1.38k stars 346 forks source link

Soft keys appear/disappear on every keystroke on password field #290

Closed 3flex closed 5 years ago

3flex commented 6 years ago

This is an odd one, but hopefully easy to pin down since it just started happening after the last Google Play release (I'm on 2.2.2.4).

I use a Moto G5 Plus, which comes with a "Moto" app that allows me to replace the on-screen nav buttons (back, home, recent apps) with gestures. Some apps are slightly buggy and even with this option on, the on-screen nav buttons will occasionally display. It's usually not disruptive so I ignore it since it's an OEM option and not part of standard Android.

However KeePassDroid as of 2.2.2.4 now toggles the on-screen buttons on every keystroke. That means every time I press a keyboard button to enter my password the keyboard jumps up or down so I have to reorient before pressing the next button. It's extremely annoying and has never happened before in any other app nor in KeePassDroid prior to 2.2.2.4.

Hopefully it's a simple fix!

3flex commented 6 years ago

I've played around a bit, and I suspect it's related to handling of the fingerprint functionality.

In other apps that support fingerprint authentication the on-screen nav buttons reappear when the system is waiting on a fingerprint. On this phone the fingerprint sensor is also used for navigation gestures, so when the system is waiting on a fingerprint it shows the on-screen nav buttons so it's still possible to go back, home or show recent apps.

It's surely something that happens in this afterTextChanged callback on the password field: https://github.com/bpellin/keepassdroid/blob/v2.2.2.4/app/src/main/java/com/keepassdroid/PasswordActivity.java#L292

I quickly scanned the code that's called but I don't have experience with fingerprint authentication implementations on Android so unfortunately I didn't make further progress, but I see a lot of toggles and stop/start listening calls which presumably cause this issue.

Xavron commented 6 years ago

It looks like this version does have the listen for fingerprints for resume integrated to fix the fingerprint not working there so that could be the trigger here (in any case that means that it should still trigger on fresh start of the app anyway).

But that's kind of messed up QUOTE "Note: The fingerprint sensor is used for authentication in critical use cases, so services must carefully design their user's experience when performing gestures on the sensor. When the sensor is in use by an app, for example, when authenticating or enrolling a user, the sensor will not detect gestures."

Repeat of the last part "When the sensor is in use by an app, for example, when authenticating or enrolling a user, the sensor will not detect gestures." ಠ▃ಠ

https://developer.android.com/reference/android/accessibilityservice/FingerprintGestureController

The kind of goes with the statement found at https://stackoverflow.com/questions/35054405/detect-gestures-on-fingerprint-sensor-android about not really getting useful swipe information with an app from it.

With FingerprintManager I only see authentication. So it looks to me as its a problem from with Moto using the sensor when an app is authenticating.

There's quite a few options though

1) Disable resume listening for Moto devices 2) Disable resume listening and go back to the broken way it was for all devices 3) Don't listen until user presses a button (Eg the fingerprint image) 4) Additional research might show some way to override the swipe or some flag to force it not to be used that I have yet to come across (maybe implementing FingerprintGestureController will stop the problem but its not valid use here and I've no way to test anything since my only fingerprint device doesn't do gestures here)

I just don't see anything else at this time.

Xavron commented 6 years ago

Adding another reply about the Moto G5 Play

It doesn't make sense to me that using the on screen keyboard will affect fingerprint swiping at all anyway.

It requires actual use of the fingerprint sensor which is found below the screen on the front: https://www.youtube.com/watch?v=ra88wwhS5AU

That means that it should not be swiping if the user is not touching it and it is then a rather glaring flaw from Moto that the user is typing when their hand is brushing against the sensor.

Moto, you're killing me :-1: lol

Xavron commented 6 years ago

Perhaps trying to hide the navigation buttons is another option: https://developer.android.com/training/system-ui/navigation

That just depends if Moto overrides it however and seems to be a rather poor workaround I think

Is there even anything else?

It doesn't seem to be a view focus issue at least on my side.

Xavron commented 6 years ago

Possible fix?

It looks like Moto's one button navi bar setting should fix this as enabling it is supposed to disable the on screen navigation bar

https://www.androidcentral.com/one-button-nav-moto-g5-exciting-new-way-interact-your-phone

"When you turn it off, you’ll see navigation icons at the bottom of all screens again."

https://motorola-global-portal.custhelp.com/app/answers/indevice_detail/a_id/116532/p/30,6720,10165

That's my final answer. I'd done spamming emails today lol

3flex commented 6 years ago

Thanks, I'm aware I can turn the feature off, but I'd prefer not to, and other apps behave correctly so it's specific to this app and a regression from the previous version published to Play Store. I'd like to help fix it but I'd have to learn both how to implement finger print auth in Android, and then how this app implemented it, so hoping someone more knowledgeable can see if they know what the fix is.

Fingerprint auth doesn't work at all for me in this app either, probably because the phone doesn't have a separate fingerprint scanner (I think there's another issue open for this).

Xavron commented 6 years ago

Fingerprint auth doesn't work at all for me in this app either, probably because the phone doesn't have a separate fingerprint scanner (I think there's another issue open for this).

Currently it goes:

onResume()
checkAvailability()
...
if (!fingerPrintHelper.isHardwareDetected()) { }  <----- no

// fingerprint is available but not configured show icon but in disabled state with some information
else if (!fingerPrintHelper.hasEnrolledFingerprints()) { } <---- *********

// finally fingerprint available and configured so we can use it
else { }
    public boolean hasEnrolledFingerprints() {
        // fingerprint hardware supported and api level OK
        return isHardwareDetected()
                // fingerprints enrolled
                && fingerprintManager != null
                && fingerprintManager.hasEnrolledFingerprints()
                // and lockscreen configured
                && KeyguardManagerCompat.isKeyguardSecure(keyguardManager);
    }

In that case, it looks like its not checking if fingerprint has been used successfully and so it thinks you can use it.

A workaround then may be to either check if successful at least once and then fail to listen from resume on all further attempts?

But regardless of whether or not fingerprint is working that does not solve the bouncing problems of the navigation bar with your Moto device "However KeePassDroid as of 2.2.2.4 now toggles the on-screen buttons on every keystroke"

The fact is that listening for fingerprints is an authentication method and should not trigger on-screen buttons. This is confusing in-of-itself unless its not the actual problem. I don't know what other apps are doing differently with fingerprint unless its not the cause.

Xavron commented 6 years ago

Fingerprint auth doesn't work at all for me in this app either

If I'm not going crazy and seeing things:

I only just noticed (in particular because I personally avoid all compat classes so didn't think of it) that KeePassDroid is using FingerprintManagerCompat

https://stackoverflow.com/questions/42575967/fingerprintmanagercompat-ishardwaredetected-return-false-in-targetapi-25

It looks like the G5 is on Android 7.1 API 25 which as mentioned in the above link can fail to provide fingerprint support using the compat class.

FingerprintManagerCompat is known to have problems https://github.com/jariz/react-native-fingerprint-android/issues/1

This could even be the cause of other devices not working as well. Which is seems to come back to FEATURE_FINGERPRINT which some devices for whatever reason do not support.

The fact is that listening for fingerprints is an authentication method and should not trigger on-screen buttons.

Will this resolve the nav bar problem here? Idk.

My last thought of that was the targetSdkVersion 12 could have a problem there as if the OS is using Immersive mode then that was introduced in API 19 and I'm not familiar with low target sdk versions but that they can at times break UI. (There is a post out there that shows the nav bar can break with lower sdk version like this app - though it wasn't applied the same way.)

But, I've now tested FingerprintManager on Samsung A7 with Oreo and it works fine. Should take about 10 minutes to implement the changes but it opens up another issue due to minSdkVersion 4 so they would have to use build flags and check for version. And perhaps other stuff.

It just might also be that Oreo update for the G5 will resolve the issue for you (or it won't)

In the end, I still don't have a real device test this with to know if it truly works or not. But, its my last option. Don't think I can find anything else on this subject without a test device.

Despite the below post, I'm sticking with these two unless someone can prove that its not. Other than this, I'm going with a problem on the device side as fingerprint API should not cause system nav bar to appear - that is not what the API is for.

I will not reply to this thread.

3flex commented 6 years ago

I doubt it affects any of your findings, but my device is on 7.0 (API 24) not 7.1.

I appreciate your efforts in looking into this!

3flex commented 5 years ago

My device was just updated to Oreo 8.1 - the problem has gone away.