Fixed accessibility issues in Wi-Fi password view in Settings
- Keep the Save button enabled at all times
- Show "*required" or "The password is invalid" to remind the user
- Show "Password (optional)" to indicate that it can remain unchanged, when modifying Wi-Fi configuration
Bug: 386897596
Bug: 402694144
Flag: EXEMPT bugfix
Test: Manual testing
atest SettingsUnitTests:AddNetworkFragmentTest
atest WifiConfigControllerTest \
WifiConfigController2Test \
WifiDialogActivityTest
Change-Id: I09b7684674ff376139565fcc196cde8d8d20a864
This commit is contained in:
@@ -2448,6 +2448,8 @@
|
||||
<string name="wifi_eap_anonymous">Anonymous identity</string>
|
||||
<!-- Label for the password of the secured network -->
|
||||
<string name="wifi_password">Password*</string>
|
||||
<!-- Label for the optional password of the secured network -->
|
||||
<string name="wifi_password_optional">Password (optional)</string>
|
||||
<!-- Label for the password of the secured network -->
|
||||
<string name="wifi_password_invalid">The password is invalid</string>
|
||||
<!-- Label for the check box to show password -->
|
||||
|
||||
@@ -206,7 +206,7 @@ public class AddNetworkFragment extends InstrumentedFragment implements WifiConf
|
||||
|
||||
@VisibleForTesting
|
||||
void handleSubmitAction() {
|
||||
if (!mUIController.canFinish()) {
|
||||
if (!mUIController.getValidator().validate()) {
|
||||
return;
|
||||
}
|
||||
successfullyFinish(mUIController.getConfig());
|
||||
|
||||
@@ -203,6 +203,9 @@ public class ConfigureWifiEntryFragment extends InstrumentedFragment implements
|
||||
|
||||
@VisibleForTesting
|
||||
void handleSubmitAction() {
|
||||
if (!mUiController.getValidator().validate()) {
|
||||
return;
|
||||
}
|
||||
final Intent intent = new Intent();
|
||||
final Activity activity = getActivity();
|
||||
intent.putExtra(NETWORK_CONFIG_KEY, mUiController.getConfig());
|
||||
|
||||
@@ -296,7 +296,7 @@ public class WifiConfigController implements TextWatcher,
|
||||
|
||||
mSsidInput = new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid,
|
||||
R.string.wifi_ssid_hint);
|
||||
mPasswordInput = new WifiPasswordInput(mView);
|
||||
mPasswordInput = new WifiPasswordInput(mView, mAccessPointSecurity);
|
||||
mValidator.addTextInput(mSsidInput);
|
||||
mValidator.addTextInput(mPasswordInput);
|
||||
|
||||
|
||||
@@ -78,6 +78,8 @@ import com.android.settings.wifi.details2.WifiPrivacyPreferenceController;
|
||||
import com.android.settings.wifi.details2.WifiPrivacyPreferenceController2;
|
||||
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.wifi.flags.Flags;
|
||||
@@ -229,7 +231,10 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
private final boolean mHideMeteredAndPrivacy;
|
||||
private final WifiManager mWifiManager;
|
||||
private final AndroidKeystoreAliasLoader mAndroidKeystoreAliasLoader;
|
||||
|
||||
private TextInputValidator mValidator = new TextInputValidator();
|
||||
private TextInputGroup mSsidInputGroup;
|
||||
private WifiPasswordInput mPasswordInput;
|
||||
|
||||
private final Context mContext;
|
||||
|
||||
@@ -301,6 +306,10 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
|
||||
mSsidInputGroup = new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid,
|
||||
R.string.wifi_ssid_hint);
|
||||
mPasswordInput = new WifiPasswordInput(mView, mWifiEntrySecurity);
|
||||
mValidator.addTextInput(mSsidInputGroup);
|
||||
mValidator.addTextInput(mPasswordInput);
|
||||
|
||||
mSsidScanButton = (ImageButton) mView.findViewById(R.id.ssid_scanner_button);
|
||||
mIpSettingsSpinner = (Spinner) mView.findViewById(R.id.ip_settings);
|
||||
mIpSettingsSpinner.setOnItemSelectedListener(this);
|
||||
@@ -519,45 +528,8 @@ public class WifiConfigController2 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() <= 128) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
boolean isSubmittable() {
|
||||
boolean enabled = false;
|
||||
boolean passwordInvalid = false;
|
||||
if (mPasswordView != null
|
||||
&& ((mWifiEntrySecurity == WifiEntry.SECURITY_WEP
|
||||
&& mPasswordView.length() == 0)
|
||||
|| (mWifiEntrySecurity == WifiEntry.SECURITY_PSK
|
||||
&& !isValidPsk(mPasswordView.getText().toString()))
|
||||
|| (mWifiEntrySecurity == WifiEntry.SECURITY_SAE
|
||||
&& !isValidSaePassword(mPasswordView.getText().toString())))) {
|
||||
passwordInvalid = true;
|
||||
}
|
||||
if ((mWifiEntry == null || !mWifiEntry.isSaved()) && passwordInvalid) {
|
||||
// If WifiEntry is not saved, apply passwordInvalid check
|
||||
enabled = false;
|
||||
} else if (mWifiEntry != null && mWifiEntry.isSaved() && passwordInvalid
|
||||
&& mPasswordView.length() > 0) {
|
||||
// If WifiEntry is saved (modifying network) and password is changed, apply
|
||||
// Invalid password check
|
||||
enabled = false;
|
||||
} else {
|
||||
enabled = ipAndProxyFieldsAreValid();
|
||||
}
|
||||
boolean enabled = ipAndProxyFieldsAreValid();
|
||||
if ((mWifiEntrySecurity == WifiEntry.SECURITY_EAP
|
||||
|| mWifiEntrySecurity == WifiEntry.SECURITY_EAP_WPA3_ENTERPRISE
|
||||
|| mWifiEntrySecurity == WifiEntry.SECURITY_EAP_SUITE_B)
|
||||
@@ -589,14 +561,6 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
return enabled;
|
||||
}
|
||||
|
||||
boolean canFinish() {
|
||||
if (!mSsidInputGroup.validate()) {
|
||||
Log.w(TAG, "Can't finish because SSID is invalid!");
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
void showWarningMessagesIfAppropriate() {
|
||||
mView.findViewById(R.id.no_user_cert_warning).setVisibility(View.GONE);
|
||||
mView.findViewById(R.id.no_domain_warning).setVisibility(View.GONE);
|
||||
@@ -1050,7 +1014,9 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
.setOnCheckedChangeListener(this);
|
||||
|
||||
if (mWifiEntry != null && mWifiEntry.isSaved()) {
|
||||
mPasswordView.setHint(R.string.wifi_unchanged);
|
||||
mPasswordInput.setCanBeEmpty(true);
|
||||
mPasswordInput.getLayout().setHint(R.string.wifi_password_optional);
|
||||
mPasswordInput.setHelperText(mContext.getString(R.string.wifi_unchanged));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1763,6 +1729,7 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
// Convert menu position to actual Wi-Fi security type
|
||||
mWifiEntrySecurity = mSecurityInPosition[position];
|
||||
showSecurityFields(/* refreshEapMethods */ true, /* refreshCertificates */ true);
|
||||
mPasswordInput.setSecurity(mWifiEntrySecurity);
|
||||
|
||||
if (WifiDppUtils.isSupportEnrolleeQrCodeScanner(mContext, mWifiEntrySecurity)) {
|
||||
mSsidScanButton.setVisibility(View.VISIBLE);
|
||||
@@ -2013,4 +1980,11 @@ public class WifiConfigController2 implements TextWatcher,
|
||||
spinner.setAdapter(getSpinnerAdapter(stringArray));
|
||||
return spinner;
|
||||
}
|
||||
|
||||
/**
|
||||
* Provides a validator to verify that the Wi-Fi configuration is ready.
|
||||
*/
|
||||
public TextInputValidator getValidator() {
|
||||
return mValidator;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,7 +118,7 @@ public class WifiDialog extends AlertDialog implements WifiConfigUiBase,
|
||||
mController.hideForgetButton();
|
||||
}
|
||||
|
||||
mDialogHelper = new WifiDialogHelper(this, mController.getValidator());
|
||||
mDialogHelper = new WifiDialogHelper(this, this, mController.getValidator());
|
||||
}
|
||||
|
||||
@SuppressWarnings("MissingSuperCall") // TODO: Fix me
|
||||
@@ -159,6 +159,9 @@ public class WifiDialog extends AlertDialog implements WifiConfigUiBase,
|
||||
public void onClick(DialogInterface dialogInterface, int id) {
|
||||
if (mListener != null) {
|
||||
switch (id) {
|
||||
case BUTTON_SUBMIT:
|
||||
mListener.onSubmit(this);
|
||||
break;
|
||||
case BUTTON_FORGET:
|
||||
if (WifiUtils.isNetworkLockedDown(getContext(), mAccessPoint.getConfig())) {
|
||||
RestrictedLockUtils.sendShowAdminSupportDetailsIntent(getContext(),
|
||||
@@ -171,11 +174,6 @@ public class WifiDialog extends AlertDialog implements WifiConfigUiBase,
|
||||
}
|
||||
}
|
||||
|
||||
/** Return true to tell the parent activity to call onSubmit before onDismiss. */
|
||||
public boolean shouldSubmitBeforeFinish() {
|
||||
return mDialogHelper.isPositive();
|
||||
}
|
||||
|
||||
@Override
|
||||
public int getMode() {
|
||||
return mMode;
|
||||
|
||||
@@ -28,6 +28,7 @@ import android.widget.TextView
|
||||
import androidx.annotation.OpenForTesting
|
||||
import androidx.appcompat.app.AlertDialog
|
||||
import com.android.settings.R
|
||||
import com.android.settings.wifi.utils.WifiDialogHelper
|
||||
import com.android.settingslib.RestrictedLockUtils
|
||||
import com.android.settingslib.RestrictedLockUtilsInternal
|
||||
import com.android.wifitrackerlib.WifiEntry
|
||||
@@ -68,6 +69,7 @@ open class WifiDialog2 @JvmOverloads constructor(
|
||||
|
||||
private lateinit var view: View
|
||||
private lateinit var controller: WifiConfigController2
|
||||
private lateinit var dialogHelper: WifiDialogHelper
|
||||
|
||||
override fun getController(): WifiConfigController2 = controller
|
||||
|
||||
@@ -89,6 +91,7 @@ open class WifiDialog2 @JvmOverloads constructor(
|
||||
if (wifiEntry == null) {
|
||||
controller.hideForgetButton()
|
||||
}
|
||||
dialogHelper = WifiDialogHelper(this, this, controller.validator)
|
||||
}
|
||||
|
||||
private fun setWindowsOverlay() {
|
||||
|
||||
@@ -346,9 +346,6 @@ public class WifiDialogActivity extends ObservableActivity implements WifiDialog
|
||||
@Override
|
||||
public void onDismiss(DialogInterface dialogInterface) {
|
||||
mDialog2 = null;
|
||||
if (mDialog != null && mDialog.shouldSubmitBeforeFinish()) {
|
||||
onSubmit(mDialog);
|
||||
}
|
||||
mDialog = null;
|
||||
finish();
|
||||
}
|
||||
|
||||
@@ -17,16 +17,18 @@
|
||||
package com.android.settings.wifi.utils
|
||||
|
||||
import android.content.DialogInterface
|
||||
import android.content.DialogInterface.BUTTON_POSITIVE
|
||||
import android.util.Log
|
||||
import androidx.appcompat.app.AlertDialog
|
||||
|
||||
open class AlertDialogHelper(val alertDialog: AlertDialog) {
|
||||
|
||||
var isPositive = false
|
||||
open class AlertDialogHelper(
|
||||
val alertDialog: AlertDialog,
|
||||
val onClickListener: DialogInterface.OnClickListener? = null,
|
||||
) {
|
||||
|
||||
init {
|
||||
alertDialog.setOnShowListener {
|
||||
alertDialog.getButton(DialogInterface.BUTTON_POSITIVE)?.setOnClickListener {
|
||||
alertDialog.getButton(BUTTON_POSITIVE)?.setOnClickListener {
|
||||
onPositiveButtonClicked()
|
||||
} ?: Log.e(TAG, "Can't get the positive button!")
|
||||
}
|
||||
@@ -37,7 +39,7 @@ open class AlertDialogHelper(val alertDialog: AlertDialog) {
|
||||
Log.w(TAG, "Can't dismiss dialog!")
|
||||
return
|
||||
}
|
||||
isPositive = true
|
||||
onClickListener?.onClick(alertDialog, BUTTON_POSITIVE)
|
||||
alertDialog.dismiss()
|
||||
}
|
||||
|
||||
|
||||
@@ -75,6 +75,7 @@ open class TextInputGroup(
|
||||
get() = layout.helperText?.toString() ?: ""
|
||||
set(value) {
|
||||
layout.setHelperText(value)
|
||||
if (value.isEmpty()) layout.isHelperTextEnabled = false
|
||||
}
|
||||
|
||||
var error: String
|
||||
@@ -85,6 +86,8 @@ open class TextInputGroup(
|
||||
}
|
||||
|
||||
open fun validate(): Boolean {
|
||||
if (!editText.isShown) return true
|
||||
|
||||
val isValid = text.isNotEmpty()
|
||||
if (!isValid) {
|
||||
Log.w(TAG, "validate failed in ${layout.hint ?: "unknown"}")
|
||||
|
||||
@@ -16,13 +16,14 @@
|
||||
|
||||
package com.android.settings.wifi.utils
|
||||
|
||||
import android.util.Log
|
||||
import android.content.DialogInterface
|
||||
import androidx.appcompat.app.AlertDialog
|
||||
|
||||
class WifiDialogHelper(
|
||||
alertDialog: AlertDialog,
|
||||
onClickListener: DialogInterface.OnClickListener?,
|
||||
private val validator: TextInputValidator,
|
||||
) : AlertDialogHelper(alertDialog) {
|
||||
) : AlertDialogHelper(alertDialog, onClickListener) {
|
||||
|
||||
override fun canDismiss(): Boolean = validator.validate()
|
||||
|
||||
|
||||
@@ -27,13 +27,16 @@ 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) {
|
||||
class WifiPasswordInput(
|
||||
view: View,
|
||||
var security: Int = SECURITY_NONE,
|
||||
) : TextInputGroup(view, R.id.password_input_layout, R.id.password, R.string.wifi_field_required) {
|
||||
|
||||
var security: Int = SECURITY_NONE
|
||||
var canBeEmpty: Boolean = false
|
||||
|
||||
override fun validate(): Boolean {
|
||||
if (!editText.isShown) return true
|
||||
if (canBeEmpty && text.isEmpty()) return true
|
||||
|
||||
return when (security) {
|
||||
SECURITY_WEP -> super.validate()
|
||||
|
||||
@@ -193,52 +193,6 @@ public class WifiConfigController2Test {
|
||||
.isEqualTo(View.GONE);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void isSubmittable_longPsk_shouldReturnFalse() {
|
||||
createController(mWifiEntry, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
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() {
|
||||
createController(mWifiEntry, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
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() {
|
||||
createController(mWifiEntry, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
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() {
|
||||
createController(mWifiEntry, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
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() {
|
||||
createController(mWifiEntry, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
final TextView password = mView.findViewById(R.id.password);
|
||||
assertThat(password).isNotNull();
|
||||
password.setText("");
|
||||
when(mWifiEntry.isSaved()).thenReturn(true);
|
||||
assertThat(mController.isSubmittable()).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void isSubmittable_nullWifiEntry_noException() {
|
||||
createController(null, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
@@ -1039,24 +993,6 @@ public class WifiConfigController2Test {
|
||||
verify(mController.mEapAnonymousView, never()).setText(any(String.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void canFinish_ssidIsEmpty_returnFalse() {
|
||||
createController(null, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
TextView ssid = mView.findViewById(R.id.ssid);
|
||||
ssid.setText("");
|
||||
|
||||
assertThat(mController.canFinish()).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void canFinish_ssidIsGood_returnTrue() {
|
||||
createController(null, WifiConfigUiBase2.MODE_CONNECT, false);
|
||||
TextView ssid = mView.findViewById(R.id.ssid);
|
||||
ssid.setText(GOOD_SSID);
|
||||
|
||||
assertThat(mController.canFinish()).isTrue();
|
||||
}
|
||||
|
||||
private void setUpModifyingSavedCertificateConfigController(String savedCaCertificate,
|
||||
String savedUserCertificate) {
|
||||
final WifiConfiguration mockWifiConfig = spy(new WifiConfiguration());
|
||||
|
||||
@@ -349,14 +349,4 @@ public class WifiDialogActivityTest {
|
||||
|
||||
verify(mActivity).dismissDialog();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void onDismiss_shouldSubmitBeforeFinish_callOnSubmit() {
|
||||
mActivity.mDialog = mWifiDialog;
|
||||
when(mWifiDialog.shouldSubmitBeforeFinish()).thenReturn(true);
|
||||
|
||||
mActivity.onDismiss(mWifiDialog);
|
||||
|
||||
verify(mActivity).onSubmit(mWifiDialog);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user