From 351e6a94e0a805bae68a878cf21da5ef7e6a0e47 Mon Sep 17 00:00:00 2001 From: Weng Su Date: Wed, 12 Mar 2025 20:46:49 +0800 Subject: [PATCH] Fixed accessibility issues in Wi-Fi password view for SUW - Keep the Save button enabled at all times - Show "*required" or "The password is invalid" to remind the user Bug: 386897596 Flag: EXEMPT bugfix Test: Manual testing atest WifiConfigControllerTest Change-Id: I442d2f958efd85f3c92309d0bed7cd3aa9ec9876 --- res/layout/wifi_network_config.xml | 4 + res/values/strings.xml | 8 +- .../settings/wifi/WifiConfigController.java | 74 +++++++------------ src/com/android/settings/wifi/WifiDialog.java | 5 +- .../settings/wifi/utils/TextInputGroup.kt | 4 +- .../settings/wifi/utils/TextInputValidator.kt | 35 +++++++++ .../settings/wifi/utils/WifiDialogHelper.kt | 8 +- .../settings/wifi/utils/WifiPasswordInput.kt | 70 ++++++++++++++++++ .../wifi/WifiConfigControllerTest.java | 41 ---------- 9 files changed, 146 insertions(+), 103 deletions(-) create mode 100644 src/com/android/settings/wifi/utils/TextInputValidator.kt create mode 100644 src/com/android/settings/wifi/utils/WifiPasswordInput.kt diff --git a/res/layout/wifi_network_config.xml b/res/layout/wifi_network_config.xml index 8319b1a3663..2c747257b0f 100644 --- a/res/layout/wifi_network_config.xml +++ b/res/layout/wifi_network_config.xml @@ -62,6 +62,8 @@ android:layout_weight="1" android:hint="@string/wifi_ssid" android:textDirection="locale" + app:helperTextEnabled="true" + app:helperText="@string/wifi_field_required" app:endIconMode="clear_text"> expand - Network name + Network name* Enter the SSID @@ -2429,7 +2429,9 @@ Anonymous identity - Password + Password* + + The password is invalid Show password @@ -2562,6 +2564,8 @@ To improve location accuracy and for other purposes, an unknown app wants to turn on network scanning, even when Wi\u2011Fi is off.\n\nAllow this for all apps that want to scan? Allow Deny + + *required This network has no internet access. Stay connected? diff --git a/src/com/android/settings/wifi/WifiConfigController.java b/src/com/android/settings/wifi/WifiConfigController.java index 713dfb2458a..003c2afed49 100644 --- a/src/com/android/settings/wifi/WifiConfigController.java +++ b/src/com/android/settings/wifi/WifiConfigController.java @@ -74,6 +74,9 @@ import com.android.settings.network.SubscriptionUtil; import com.android.settings.utils.AndroidKeystoreAliasLoader; import com.android.settings.wifi.details2.WifiPrivacyPreferenceController; import com.android.settings.wifi.dpp.WifiDppUtils; +import com.android.settings.wifi.utils.TextInputGroup; +import com.android.settings.wifi.utils.TextInputValidator; +import com.android.settings.wifi.utils.WifiPasswordInput; import com.android.settingslib.Utils; import com.android.settingslib.utils.ThreadUtils; import com.android.settingslib.wifi.AccessPoint; @@ -215,7 +218,10 @@ public class WifiConfigController implements TextWatcher, private String[] mLevels; private int mMode; - private TextView mSsidView; + + private TextInputValidator mValidator = new TextInputValidator(); + private TextInputGroup mSsidInput; + private WifiPasswordInput mPasswordInput; private Context mContext; @@ -288,6 +294,12 @@ public class WifiConfigController implements TextWatcher, wepWarningLayout.setVisibility(View.VISIBLE); } + mSsidInput = new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid, + R.string.wifi_ssid_hint); + mPasswordInput = new WifiPasswordInput(mView); + mValidator.addTextInput(mSsidInput); + mValidator.addTextInput(mPasswordInput); + mSsidScanButton = (ImageButton) mView.findViewById(R.id.ssid_scanner_button); mIpSettingsSpinner = (Spinner) mView.findViewById(R.id.ip_settings); mIpSettingsSpinner.setOnItemSelectedListener(this); @@ -517,45 +529,8 @@ public class WifiConfigController implements TextWatcher, submit.setEnabled(isSubmittable()); } - boolean isValidPsk(String password) { - if (password.length() == 64 && password.matches("[0-9A-Fa-f]{64}")) { - return true; - } else if (password.length() >= 8 && password.length() <= 63) { - return true; - } - return false; - } - - boolean isValidSaePassword(String password) { - if (password.length() >= 1 && password.length() <= 63) { - return true; - } - return false; - } - boolean isSubmittable() { - boolean enabled = false; - boolean passwordInvalid = false; - if (mPasswordView != null - && ((mAccessPointSecurity == AccessPoint.SECURITY_WEP - && mPasswordView.length() == 0) - || (mAccessPointSecurity == AccessPoint.SECURITY_PSK - && !isValidPsk(mPasswordView.getText().toString())) - || (mAccessPointSecurity == AccessPoint.SECURITY_SAE - && !isValidSaePassword(mPasswordView.getText().toString())))) { - passwordInvalid = true; - } - if ((mAccessPoint == null || !mAccessPoint.isSaved()) && passwordInvalid) { - // If Accesspoint is not saved, apply passwordInvalid check - enabled = false; - } else if (mAccessPoint != null && mAccessPoint.isSaved() && passwordInvalid - && mPasswordView.length() > 0) { - // If AccessPoint is saved (modifying network) and password is changed, apply - // Invalid password check - enabled = false; - } else { - enabled = ipAndProxyFieldsAreValid(); - } + boolean enabled = ipAndProxyFieldsAreValid(); if ((mAccessPointSecurity == AccessPoint.SECURITY_EAP || mAccessPointSecurity == AccessPoint.SECURITY_EAP_WPA3_ENTERPRISE || mAccessPointSecurity == AccessPoint.SECURITY_EAP_SUITE_B) @@ -592,11 +567,8 @@ public class WifiConfigController implements TextWatcher, mView.findViewById(R.id.no_domain_warning).setVisibility(View.GONE); mView.findViewById(R.id.ssid_too_long_warning).setVisibility(View.GONE); - if (mSsidView != null) { - final String ssid = mSsidView.getText().toString(); - if (WifiUtils.isSSIDTooLong(ssid)) { - mView.findViewById(R.id.ssid_too_long_warning).setVisibility(View.VISIBLE); - } + if (WifiUtils.isSSIDTooLong(mSsidInput.getText())) { + mView.findViewById(R.id.ssid_too_long_warning).setVisibility(View.VISIBLE); } if (mEapCaCertSpinner != null && mView.findViewById(R.id.l_ca_cert).getVisibility() != View.GONE) { @@ -626,8 +598,7 @@ public class WifiConfigController implements TextWatcher, WifiConfiguration config = new WifiConfiguration(); if (mAccessPoint == null) { - config.SSID = AccessPoint.convertToQuotedString( - mSsidView.getText().toString()); + config.SSID = AccessPoint.convertToQuotedString(mSsidInput.getText()); // If the user adds a network manually, assume that it is hidden. config.hiddenSSID = mHiddenSettingsSpinner.getSelectedItemPosition() == HIDDEN_NETWORK; } else if (!mAccessPoint.isSaved()) { @@ -1677,6 +1648,7 @@ public class WifiConfigController implements TextWatcher, // Convert menu position to actual Wi-Fi security type mAccessPointSecurity = mSecurityInPosition[position]; showSecurityFields(/* refreshEapMethods */ true, /* refreshCertificates */ true); + mPasswordInput.setSecurity(mAccessPointSecurity); if (WifiDppUtils.isSupportEnrolleeQrCodeScanner(mContext, mAccessPointSecurity)) { mSsidScanButton.setVisibility(View.VISIBLE); @@ -1725,8 +1697,7 @@ public class WifiConfigController implements TextWatcher, private void configureSecuritySpinner() { mConfigUi.setTitle(R.string.wifi_add_network); - mSsidView = (TextView) mView.findViewById(R.id.ssid); - mSsidView.addTextChangedListener(this); + mSsidInput.addTextChangedListener(this); mSecuritySpinner = ((Spinner) mView.findViewById(R.id.security)); mSecuritySpinner.setOnItemSelectedListener(this); @@ -1894,4 +1865,11 @@ public class WifiConfigController implements TextWatcher, } }); } + + /** + * Provides a validator to verify that the Wi-Fi configuration is ready. + */ + public TextInputValidator getValidator() { + return mValidator; + } } diff --git a/src/com/android/settings/wifi/WifiDialog.java b/src/com/android/settings/wifi/WifiDialog.java index 38c99b6a759..2fd3ce1ea93 100644 --- a/src/com/android/settings/wifi/WifiDialog.java +++ b/src/com/android/settings/wifi/WifiDialog.java @@ -28,7 +28,6 @@ import android.widget.TextView; import androidx.appcompat.app.AlertDialog; import com.android.settings.R; -import com.android.settings.wifi.utils.TextInputGroup; import com.android.settings.wifi.utils.WifiDialogHelper; import com.android.settingslib.RestrictedLockUtils; import com.android.settingslib.RestrictedLockUtilsInternal; @@ -119,9 +118,7 @@ public class WifiDialog extends AlertDialog implements WifiConfigUiBase, mController.hideForgetButton(); } - mDialogHelper = new WifiDialogHelper(this, - new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid, - R.string.vpn_field_required)); + mDialogHelper = new WifiDialogHelper(this, mController.getValidator()); } @SuppressWarnings("MissingSuperCall") // TODO: Fix me diff --git a/src/com/android/settings/wifi/utils/TextInputGroup.kt b/src/com/android/settings/wifi/utils/TextInputGroup.kt index 22fb001e040..3fe2d9d08a4 100644 --- a/src/com/android/settings/wifi/utils/TextInputGroup.kt +++ b/src/com/android/settings/wifi/utils/TextInputGroup.kt @@ -25,7 +25,7 @@ import com.google.android.material.textfield.TextInputLayout /** A widget that wraps the relationship work between a TextInputLayout and an EditText. */ open class TextInputGroup( - private val view: View, + val view: View, private val layoutId: Int, private val editTextId: Int, private val errorMessageId: Int, @@ -88,7 +88,7 @@ open class TextInputGroup( val isValid = text.isNotEmpty() if (!isValid) { Log.w(TAG, "validate failed in ${layout.hint ?: "unknown"}") - error = errorMessage.toString() + error = errorMessage } return isValid } diff --git a/src/com/android/settings/wifi/utils/TextInputValidator.kt b/src/com/android/settings/wifi/utils/TextInputValidator.kt new file mode 100644 index 00000000000..ea3e1464283 --- /dev/null +++ b/src/com/android/settings/wifi/utils/TextInputValidator.kt @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.wifi.utils + +/** + * The validator used to validate all {@code TextInputGroup} at the same time + */ +class TextInputValidator { + + private val textInputList: MutableList = ArrayList() + + fun addTextInput(textInputGroup: TextInputGroup) { + textInputList += textInputGroup + } + + fun validate(): Boolean { + var isValidate = true + for (input in textInputList) if (!input.validate()) isValidate = false + return isValidate + } +} \ No newline at end of file diff --git a/src/com/android/settings/wifi/utils/WifiDialogHelper.kt b/src/com/android/settings/wifi/utils/WifiDialogHelper.kt index aa41b969a6e..f4c3327e3bf 100644 --- a/src/com/android/settings/wifi/utils/WifiDialogHelper.kt +++ b/src/com/android/settings/wifi/utils/WifiDialogHelper.kt @@ -21,14 +21,10 @@ import androidx.appcompat.app.AlertDialog class WifiDialogHelper( alertDialog: AlertDialog, - private val ssidInputGroup: TextInputGroup? = null, + private val validator: TextInputValidator, ) : AlertDialogHelper(alertDialog) { - override fun canDismiss(): Boolean { - val isValid = ssidInputGroup?.validate() ?: true - if (!isValid) Log.w(TAG, "SSID is invalid!") - return isValid - } + override fun canDismiss(): Boolean = validator.validate() companion object { const val TAG = "WifiDialogHelper" diff --git a/src/com/android/settings/wifi/utils/WifiPasswordInput.kt b/src/com/android/settings/wifi/utils/WifiPasswordInput.kt new file mode 100644 index 00000000000..19ebdd9658b --- /dev/null +++ b/src/com/android/settings/wifi/utils/WifiPasswordInput.kt @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.wifi.utils + +import android.util.Log +import android.view.View +import com.android.settings.R +import com.android.wifitrackerlib.WifiEntry.SECURITY_NONE +import com.android.wifitrackerlib.WifiEntry.SECURITY_PSK +import com.android.wifitrackerlib.WifiEntry.SECURITY_SAE +import com.android.wifitrackerlib.WifiEntry.SECURITY_WEP + +/** + * The Wi-Fi password {@code TextInputGroup} that supports input validation. + */ +class WifiPasswordInput(view: View) : + TextInputGroup(view, R.id.password_input_layout, R.id.password, R.string.wifi_field_required) { + + var security: Int = SECURITY_NONE + + override fun validate(): Boolean { + if (!editText.isShown) return true + + return when (security) { + SECURITY_WEP -> super.validate() + SECURITY_PSK -> super.validate() && isValidPsk(text).also { valid -> + if (!valid) { + error = view.context.getString(R.string.wifi_password_invalid) + Log.w(TAG, "validate failed in ${layout.hint ?: "unknown"} for PSK") + } + } + + SECURITY_SAE -> super.validate() && isValidSae(text).also { valid -> + if (!valid) { + error = view.context.getString(R.string.wifi_password_invalid) + Log.w(TAG, "validate failed in ${layout.hint ?: "unknown"} for SAE") + } + } + + else -> true + } + } + + companion object { + const val TAG = "WifiPasswordInput" + + fun isValidPsk(password: String): Boolean { + return (password.length == 64 && password.matches("[0-9A-Fa-f]{64}".toRegex())) || + (password.length in 8..63) + } + + fun isValidSae(password: String): Boolean { + return password.length in 1..63 + } + } +} \ No newline at end of file diff --git a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java index d5d395e4452..a6c1573daaf 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiConfigControllerTest.java @@ -145,47 +145,6 @@ public class WifiConfigControllerTest { .isEqualTo(View.GONE); } - @Test - public void isSubmittable_longPsk_shouldReturnFalse() { - final TextView password = mView.findViewById(R.id.password); - assertThat(password).isNotNull(); - password.setText(LONG_PSK); - assertThat(mController.isSubmittable()).isFalse(); - } - - @Test - public void isSubmittable_shortPsk_shouldReturnFalse() { - final TextView password = mView.findViewById(R.id.password); - assertThat(password).isNotNull(); - password.setText(SHORT_PSK); - assertThat(mController.isSubmittable()).isFalse(); - } - - @Test - public void isSubmittable_goodPsk_shouldReturnTrue() { - final TextView password = mView.findViewById(R.id.password); - assertThat(password).isNotNull(); - password.setText(GOOD_PSK); - assertThat(mController.isSubmittable()).isTrue(); - } - - @Test - public void isSubmittable_hexPsk_shouldReturnTrue() { - final TextView password = mView.findViewById(R.id.password); - assertThat(password).isNotNull(); - password.setText(HEX_PSK); - assertThat(mController.isSubmittable()).isTrue(); - } - - @Test - public void isSubmittable_savedConfigZeroLengthPassword_shouldReturnTrue() { - final TextView password = mView.findViewById(R.id.password); - assertThat(password).isNotNull(); - password.setText(""); - when(mAccessPoint.isSaved()).thenReturn(true); - assertThat(mController.isSubmittable()).isTrue(); - } - @Test public void isSubmittable_nullAccessPoint_noException() { mController =