From 0d364c7379979ac5cd8a252136a63e0f85bb110f Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Wed, 10 May 2023 12:21:12 -0700 Subject: [PATCH 1/2] Fix pin error scenario for talkback. There were a couple of issues with the accessibility of the wrong pin scenario. 1. the user activity signal from the password view is trying to set the error message to an empty string. Move setting the error message to after resetting the password view. 2. No text is set yet while we are animating the view and this prevents the text from being announced. I remedied this by relying on a textwatcher to keep track of the textview. 3. The password view is constantly being selected, which shouldn't really be the case. This is because we set the input to be disabled as we check the password and re-enable it. While in this process, we requestfocus if focus is not on the password view. 4. For some reason we set the textview to "wrong pin" and then immediately an empty string. There is a 250ms delay to make the accessibility announcement. After 250ms, we want to ensure that the text is still displayed in the textview. Consequently, once a wrong pin in inputted, the only talkback text is now "Wrong Pin" Fixes: 281635711 Fixes: 281787557 Test: wrong pin scenario in bouncer. Test: password, simpin, and sim puk views. Change-Id: I018e067e0a3b17fbd7921f40924ced401707297c --- .../KeyguardAbsKeyInputViewController.java | 2 +- .../KeyguardMessageAreaController.java | 35 +++++++++++++++---- .../keyguard/KeyguardPinBasedInputView.java | 3 -- .../KeyguardMessageAreaControllerTest.java | 18 +++++----- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java index 510fcbfd8bee2..a229b135d1a2a 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java @@ -179,10 +179,10 @@ public abstract class KeyguardAbsKeyInputViewController private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final ConfigurationController mConfigurationController; private final AnnounceRunnable mAnnounceRunnable; + private final TextWatcher mTextWatcher = new TextWatcher() { + @Override + public void afterTextChanged(Editable editable) { + CharSequence msg = editable; + if (!TextUtils.isEmpty(msg)) { + mView.removeCallbacks(mAnnounceRunnable); + mAnnounceRunnable.setTextToAnnounce(msg); + mView.postDelayed(() -> { + if (msg == mView.getText()) { + mAnnounceRunnable.run(); + } + }, ANNOUNCEMENT_DELAY); + } + } + + @Override + public void beforeTextChanged(CharSequence charSequence, int i, int i1, int i2) { + /* no-op */ + } + + @Override + public void onTextChanged(CharSequence charSequence, int i, int i1, int i2) { + /* no-op */ + } + }; private KeyguardUpdateMonitorCallback mInfoCallback = new KeyguardUpdateMonitorCallback() { public void onFinishedGoingToSleep(int why) { @@ -89,12 +116,14 @@ public class KeyguardMessageAreaController mKeyguardUpdateMonitor.registerCallback(mInfoCallback); mView.setSelected(mKeyguardUpdateMonitor.isDeviceInteractive()); mView.onThemeChanged(); + mView.addTextChangedListener(mTextWatcher); } @Override protected void onViewDetached() { mConfigurationController.removeCallback(mConfigurationListener); mKeyguardUpdateMonitor.removeCallback(mInfoCallback); + mView.removeTextChangedListener(mTextWatcher); } /** @@ -113,12 +142,6 @@ public class KeyguardMessageAreaController */ public void setMessage(CharSequence s, boolean animate) { mView.setMessage(s, animate); - CharSequence msg = mView.getText(); - if (!TextUtils.isEmpty(msg)) { - mView.removeCallbacks(mAnnounceRunnable); - mAnnounceRunnable.setTextToAnnounce(msg); - mView.postDelayed(mAnnounceRunnable, ANNOUNCEMENT_DELAY); - } } public void setMessage(int resId) { diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java index b4ddc9a975c22..233974711d382 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPinBasedInputView.java @@ -82,9 +82,6 @@ public abstract class KeyguardPinBasedInputView extends KeyguardAbsKeyInputView protected void setPasswordEntryInputEnabled(boolean enabled) { mPasswordEntry.setEnabled(enabled); mOkButton.setEnabled(enabled); - if (enabled && !mPasswordEntry.hasFocus()) { - mPasswordEntry.requestFocus(); - } } @Override diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardMessageAreaControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardMessageAreaControllerTest.java index a35e5b59f7651..d4522d003d522 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardMessageAreaControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardMessageAreaControllerTest.java @@ -27,6 +27,8 @@ import static org.mockito.Mockito.when; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; +import android.text.Editable; +import android.text.TextWatcher; import com.android.systemui.SysuiTestCase; import com.android.systemui.statusbar.policy.ConfigurationController; @@ -93,16 +95,16 @@ public class KeyguardMessageAreaControllerTest extends SysuiTestCase { } @Test - public void testSetMessage_AnnounceForAccessibility() { - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Runnable.class); - when(mKeyguardMessageArea.getText()).thenReturn("abc"); - mMessageAreaController.setMessage("abc"); + public void textChanged_AnnounceForAccessibility() { + ArgumentCaptor textWatcherArgumentCaptor = ArgumentCaptor.forClass( + TextWatcher.class); + mMessageAreaController.onViewAttached(); + verify(mKeyguardMessageArea).addTextChangedListener(textWatcherArgumentCaptor.capture()); - verify(mKeyguardMessageArea).setMessage("abc", /* animate= */ true); + textWatcherArgumentCaptor.getValue().afterTextChanged( + Editable.Factory.getInstance().newEditable("abc")); verify(mKeyguardMessageArea).removeCallbacks(any(Runnable.class)); - verify(mKeyguardMessageArea).postDelayed(argumentCaptor.capture(), anyLong()); - argumentCaptor.getValue().run(); - verify(mKeyguardMessageArea).announceForAccessibility("abc"); + verify(mKeyguardMessageArea).postDelayed(any(Runnable.class), anyLong()); } @Test From a2614f20b466542a9cd0f5203a2ab8096fe47531 Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Thu, 11 May 2023 10:20:20 -0700 Subject: [PATCH 2/2] Fix lockout state for auto pin confirm In Autoconfirmation, if we lockout, we want to have the backspace key have the same behavior as the enter key. We also want to remove pin hinting. Fixes: 281661030 Test: autoconfirmation, long press backspace key Test: autoconfirmation lockout and regular authenticate Change-Id: Ia1128b9d2f98b1758b23e3a75909900810fd051a --- .../keyguard/KeyguardPinViewController.java | 70 +++++++++++-------- .../com/android/keyguard/NumPadButton.java | 5 ++ .../android/keyguard/PasswordTextView.java | 8 ++- .../keyguard/KeyguardPinViewControllerTest.kt | 10 ++- 4 files changed, 61 insertions(+), 32 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPinViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPinViewController.java index f23bb0ae11f69..1adaafb10e768 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPinViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPinViewController.java @@ -16,8 +16,6 @@ package com.android.keyguard; -import static com.android.systemui.keyguard.shared.constants.KeyguardBouncerConstants.DEFAULT_PIN_LENGTH; - import android.view.View; import com.android.internal.util.LatencyTracker; @@ -42,10 +40,9 @@ public class KeyguardPinViewController private NumPadButton mBackspaceKey; private View mOkButton = mView.findViewById(R.id.key_enter); - private int mUserId; private long mPinLength; - private int mPasswordFailedAttempts; + private boolean mDisabledAutoConfirmation; protected KeyguardPinViewController(KeyguardPINView view, KeyguardUpdateMonitor keyguardUpdateMonitor, @@ -84,9 +81,8 @@ public class KeyguardPinViewController protected void onUserInput() { super.onUserInput(); - if (isAutoConfirmation()) { - updateOKButtonVisibility(); - updateBackSpaceVisibility(); + if (isAutoPinConfirmEnabledInSettings()) { + updateAutoConfirmationState(); if (mPasswordEntry.getText().length() == mPinLength && mOkButton.getVisibility() == View.INVISIBLE) { verifyPasswordAndUnlock(); @@ -103,13 +99,9 @@ public class KeyguardPinViewController @Override public void startAppearAnimation() { if (mFeatureFlags.isEnabled(Flags.AUTO_PIN_CONFIRMATION)) { - mUserId = KeyguardUpdateMonitor.getCurrentUser(); - mPinLength = mLockPatternUtils.getPinLength(mUserId); - mBackspaceKey.setTransparentMode(/* isTransparentMode= */ isAutoConfirmation()); - updateOKButtonVisibility(); - updateBackSpaceVisibility(); + mPinLength = mLockPatternUtils.getPinLength(KeyguardUpdateMonitor.getCurrentUser()); mPasswordEntry.setUsePinShapes(true); - mPasswordEntry.setIsPinHinting(isAutoConfirmation() && isPinHinting()); + updateAutoConfirmationState(); } super.startAppearAnimation(); } @@ -120,13 +112,25 @@ public class KeyguardPinViewController mKeyguardUpdateMonitor.needsSlowUnlockTransition(), finishRunnable); } + @Override + protected void handleAttemptLockout(long elapsedRealtimeDeadline) { + super.handleAttemptLockout(elapsedRealtimeDeadline); + updateAutoConfirmationState(); + } + + private void updateAutoConfirmationState() { + mDisabledAutoConfirmation = mLockPatternUtils.getCurrentFailedPasswordAttempts( + KeyguardUpdateMonitor.getCurrentUser()) >= MIN_FAILED_PIN_ATTEMPTS; + updateOKButtonVisibility(); + updateBackSpaceVisibility(); + updatePinHinting(); + } /** * Updates the visibility of the OK button for auto confirm feature */ private void updateOKButtonVisibility() { - mPasswordFailedAttempts = mLockPatternUtils.getCurrentFailedPasswordAttempts(mUserId); - if (isAutoConfirmation() && mPasswordFailedAttempts < MIN_FAILED_PIN_ATTEMPTS) { + if (isAutoPinConfirmEnabledInSettings() && !mDisabledAutoConfirmation) { mOkButton.setVisibility(View.INVISIBLE); } else { mOkButton.setVisibility(View.VISIBLE); @@ -134,33 +138,41 @@ public class KeyguardPinViewController } /** - * Updates the visibility and the enabled state of the backspace. + * Updates the visibility and the enabled state of the backspace. * Visibility changes are only for auto confirmation configuration. */ private void updateBackSpaceVisibility() { - if (!isAutoConfirmation()) { - return; - } - - if (mPasswordEntry.getText().length() > 0) { - mBackspaceKey.setVisibility(View.VISIBLE); - } else { - mBackspaceKey.setVisibility(View.INVISIBLE); + boolean isAutoConfirmation = isAutoPinConfirmEnabledInSettings(); + mBackspaceKey.setTransparentMode(/* isTransparentMode= */ + isAutoConfirmation && !mDisabledAutoConfirmation); + if (isAutoConfirmation) { + if (mPasswordEntry.getText().length() > 0 + || mDisabledAutoConfirmation) { + mBackspaceKey.setVisibility(View.VISIBLE); + } else { + mBackspaceKey.setVisibility(View.INVISIBLE); + } } } + /** Updates whether to use pin hinting or not. */ + void updatePinHinting() { + mPasswordEntry.setIsPinHinting(isAutoPinConfirmEnabledInSettings() && isPinHinting() + && !mDisabledAutoConfirmation); + } /** - * Responsible for identifying if PIN hinting is to be enabled or not + * Responsible for identifying if PIN hinting is to be enabled or not */ private boolean isPinHinting() { - return mLockPatternUtils.getPinLength(mUserId) == DEFAULT_PIN_LENGTH; + return mLockPatternUtils.getPinLength(KeyguardUpdateMonitor.getCurrentUser()) + == DEFAULT_PIN_LENGTH; } /** - * Responsible for identifying if auto confirm is enabled or not in Settings + * Responsible for identifying if auto confirm is enabled or not in Settings */ - private boolean isAutoConfirmation() { + private boolean isAutoPinConfirmEnabledInSettings() { //Checks if user has enabled the auto confirm in Settings - return mLockPatternUtils.isAutoPinConfirmEnabled(mUserId); + return mLockPatternUtils.isAutoPinConfirmEnabled(KeyguardUpdateMonitor.getCurrentUser()); } } diff --git a/packages/SystemUI/src/com/android/keyguard/NumPadButton.java b/packages/SystemUI/src/com/android/keyguard/NumPadButton.java index 6ae80a62891be..ebd234fd08464 100644 --- a/packages/SystemUI/src/com/android/keyguard/NumPadButton.java +++ b/packages/SystemUI/src/com/android/keyguard/NumPadButton.java @@ -116,7 +116,12 @@ public class NumPadButton extends AlphaOptimizedImageButton implements NumPadAni * @param isTransparentMode */ public void setTransparentMode(boolean isTransparentMode) { + if (mIsTransparentMode == isTransparentMode) { + return; + } + mIsTransparentMode = isTransparentMode; + if (isTransparentMode) { setBackgroundColor(getResources().getColor(android.R.color.transparent)); } else { diff --git a/packages/SystemUI/src/com/android/keyguard/PasswordTextView.java b/packages/SystemUI/src/com/android/keyguard/PasswordTextView.java index 540001135543e..8e8ee48aba831 100644 --- a/packages/SystemUI/src/com/android/keyguard/PasswordTextView.java +++ b/packages/SystemUI/src/com/android/keyguard/PasswordTextView.java @@ -101,6 +101,7 @@ public class PasswordTextView extends FrameLayout { private Interpolator mFastOutSlowInInterpolator; private boolean mShowPassword = true; private UserActivityListener mUserActivityListener; + private boolean mIsPinHinting; private PinShapeInput mPinShapeInput; private boolean mUsePinShapes = false; @@ -419,10 +420,15 @@ public class PasswordTextView extends FrameLayout { /** * Determines whether AutoConfirmation feature is on. * - * @param usePinShapes * @param isPinHinting */ public void setIsPinHinting(boolean isPinHinting) { + // Do not reinflate the view if we are using the same one. + if (mPinShapeInput != null && mIsPinHinting == isPinHinting) { + return; + } + mIsPinHinting = isPinHinting; + if (mPinShapeInput != null) { removeView(mPinShapeInput.getView()); mPinShapeInput = null; diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardPinViewControllerTest.kt b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardPinViewControllerTest.kt index 70476aa088dc1..d3b41902499c7 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardPinViewControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardPinViewControllerTest.kt @@ -152,9 +152,15 @@ class KeyguardPinViewControllerTest : SysuiTestCase() { `when`(passwordTextView.text).thenReturn("") pinViewController.startAppearAnimation() - verify(deleteButton).visibility = View.INVISIBLE + verify(deleteButton).visibility = View.VISIBLE verify(enterButton).visibility = View.VISIBLE verify(passwordTextView).setUsePinShapes(true) - verify(passwordTextView).setIsPinHinting(true) + verify(passwordTextView).setIsPinHinting(false) + } + + @Test + fun handleLockout_readsNumberOfErrorAttempts() { + pinViewController.handleAttemptLockout(0) + verify(lockPatternUtils).getCurrentFailedPasswordAttempts(anyInt()) } }