Fixed accessibility issues in VPN Settings

- Show "(required)" and errors in required fields to alert users

- Show "(optional)" below each optional field

Bug: 386025633
Flag: EXEMPT bugfix
Test: Manual testing
  atest WifiConfigController2Test
Change-Id: Iefbd68e6658af7b073db219b3e04e94805092759

res/layout/vpn_dialog.xml

@@ -53,6 +53,8 @@
                     android:id="@+id/name_layout"
                     android:hint="@string/vpn_name"
                     app:endIconMode="clear_text"
+                    app:helperTextEnabled="true"
+                    app:helperText="@string/vpn_required"
                     app:errorEnabled="true">
                     <com.google.android.material.textfield.TextInputEditText
                         style="@style/vpn_value"
@@ -73,6 +75,8 @@
                     android:id="@+id/server_layout"
                     android:hint="@string/vpn_server"
                     app:endIconMode="clear_text"
+                    app:helperTextEnabled="true"
+                    app:helperText="@string/vpn_required"
                     app:errorEnabled="true">
                     <com.google.android.material.textfield.TextInputEditText
                         style="@style/vpn_value"
@@ -90,7 +94,7 @@
                         android:hint="@string/vpn_ipsec_identifier"
                         app:endIconMode="clear_text"
                         app:helperTextEnabled="true"
-                        app:helperText="@string/vpn_not_used"
+                        app:helperText="@string/vpn_required"
                         app:errorEnabled="true">
                         <com.google.android.material.textfield.TextInputEditText
                             style="@style/vpn_value"
@@ -108,6 +112,8 @@
                         android:id="@+id/ipsec_secret_layout"
                         android:hint="@string/vpn_ipsec_secret"
                         app:endIconMode="password_toggle"
+                        app:helperTextEnabled="true"
+                        app:helperText="@string/vpn_required"
                         app:errorEnabled="true">
                         <com.google.android.material.textfield.TextInputEditText
                             style="@style/vpn_value"
@@ -183,7 +189,7 @@
                         android:hint="@string/proxy_hostname_label"
                         app:endIconMode="clear_text"
                         app:helperTextEnabled="true"
-                        app:helperText="@string/proxy_hostname_hint"
+                        app:helperText="@string/vpn_optional"
                         app:errorEnabled="true">
                         <com.google.android.material.textfield.TextInputEditText
                             style="@style/vpn_value"
@@ -197,7 +203,7 @@
                         android:hint="@string/proxy_port_label"
                         app:endIconMode="clear_text"
                         app:helperTextEnabled="true"
-                        app:helperText="@string/proxy_port_hint"
+                        app:helperText="@string/vpn_optional"
                         app:errorEnabled="true">
                         <com.google.android.material.textfield.TextInputEditText
                             style="@style/vpn_value"
@@ -217,6 +223,8 @@
                     android:id="@+id/username_layout"
                     android:hint="@string/vpn_username"
                     app:endIconMode="clear_text"
+                    app:helperTextEnabled="true"
+                    app:helperText="@string/vpn_optional"
                     app:errorEnabled="true">
                     <com.google.android.material.textfield.TextInputEditText
                         style="@style/vpn_value"
@@ -228,6 +236,8 @@
                     android:id="@+id/password_layout"
                     android:hint="@string/vpn_password"
                     app:endIconMode="password_toggle"
+                    app:helperTextEnabled="true"
+                    app:helperText="@string/vpn_optional"
                     app:errorEnabled="true">
                     <com.google.android.material.textfield.TextInputEditText
                         style="@style/vpn_value"

res/values/strings.xml

@@ -7275,6 +7275,12 @@ Data usage charges may apply.</string>
         generic error. [CHAR LIMIT=120] -->
     <string name="vpn_always_on_invalid_reason_other">The information entered doesn\'t support
         always-on VPN</string>
+    <!-- Hint for an optional field in a VPN profile. [CHAR LIMIT=40] -->
+    <string name="vpn_optional">(optional)</string>
+    <!-- Hint for a required field in a VPN profile. [CHAR LIMIT=40] -->
+    <string name="vpn_required">(required)</string>
+    <!-- Error message displayed below the VPN EditText when the filed is required. [CHAR LIMIT=NONE] -->
+    <string name="vpn_field_required">The field is required</string>
 
     <!-- Button label to cancel changing a VPN profile. [CHAR LIMIT=40] -->
     <string name="vpn_cancel">Cancel</string>

src/com/android/settings/vpn2/ConfigDialog.java

@@ -40,6 +40,7 @@ import com.android.internal.net.VpnProfile;
 import com.android.net.module.util.ProxyUtils;
 import com.android.settings.R;
 import com.android.settings.utils.AndroidKeystoreAliasLoader;
+import com.android.settings.wifi.utils.TextInputGroup;
 
 import java.util.Collection;
 import java.util.List;
@@ -70,16 +71,17 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
 
     private View mView;
 
-    private TextView mName;
+    private TextInputGroup mNameInput;
     private Spinner mType;
-    private TextView mServer;
+    private TextInputGroup mServerInput;
-    private TextView mUsername;
+    private TextInputGroup mUsernameInput;
+    private TextInputGroup mPasswordInput;
     private TextView mPassword;
     private Spinner mProxySettings;
     private TextView mProxyHost;
     private TextView mProxyPort;
-    private TextView mIpsecIdentifier;
+    private TextInputGroup mIpsecIdentifierInput;
-    private TextView mIpsecSecret;
+    private TextInputGroup mIpsecSecretInput;
     private Spinner mIpsecUserCert;
     private Spinner mIpsecCaCert;
     private Spinner mIpsecServerCert;
@@ -106,16 +108,22 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
         Context context = getContext();
 
         // First, find out all the fields.
-        mName = (TextView) mView.findViewById(R.id.name);
+        mNameInput = new TextInputGroup(mView, R.id.name_layout, R.id.name,
+                R.string.vpn_field_required);
         mType = (Spinner) mView.findViewById(R.id.type);
-        mServer = (TextView) mView.findViewById(R.id.server);
+        mServerInput = new TextInputGroup(mView, R.id.server_layout, R.id.server,
-        mUsername = (TextView) mView.findViewById(R.id.username);
+                R.string.vpn_field_required);
-        mPassword = (TextView) mView.findViewById(R.id.password);
+        mUsernameInput = new TextInputGroup(mView, R.id.username_layout, R.id.username,
+                R.string.vpn_field_required);
+        mPasswordInput = new TextInputGroup(mView, R.id.password_layout, R.id.password,
+                R.string.vpn_field_required);
         mProxySettings = (Spinner) mView.findViewById(R.id.vpn_proxy_settings);
         mProxyHost = (TextView) mView.findViewById(R.id.vpn_proxy_host);
         mProxyPort = (TextView) mView.findViewById(R.id.vpn_proxy_port);
-        mIpsecIdentifier = (TextView) mView.findViewById(R.id.ipsec_identifier);
+        mIpsecIdentifierInput = new TextInputGroup(mView, R.id.ipsec_identifier_layout,
-        mIpsecSecret = (TextView) mView.findViewById(R.id.ipsec_secret);
+                R.id.ipsec_identifier, R.string.vpn_field_required);
+        mIpsecSecretInput = new TextInputGroup(mView, R.id.ipsec_secret_layout, R.id.ipsec_secret,
+                R.string.vpn_field_required);
         mIpsecUserCert = (Spinner) mView.findViewById(R.id.ipsec_user_cert);
         mIpsecCaCert = (Spinner) mView.findViewById(R.id.ipsec_ca_cert);
         mIpsecServerCert = (Spinner) mView.findViewById(R.id.ipsec_server_cert);
@@ -125,21 +133,21 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
         mAlwaysOnInvalidReason = (TextView) mView.findViewById(R.id.always_on_invalid_reason);
 
         // Second, copy values from the profile.
-        mName.setText(mProfile.name);
+        mNameInput.setText(mProfile.name);
         setTypesByFeature(mType);
         mType.setSelection(convertVpnProfileConstantToTypeIndex(mProfile.type));
-        mServer.setText(mProfile.server);
+        mServerInput.setText(mProfile.server);
         if (mProfile.saveLogin) {
-            mUsername.setText(mProfile.username);
+            mUsernameInput.setText(mProfile.username);
-            mPassword.setText(mProfile.password);
+            mPasswordInput.setText(mProfile.password);
         }
         if (mProfile.proxy != null) {
             mProxyHost.setText(mProfile.proxy.getHost());
             int port = mProfile.proxy.getPort();
             mProxyPort.setText(port == 0 ? "" : Integer.toString(port));
         }
-        mIpsecIdentifier.setText(mProfile.ipsecIdentifier);
+        mIpsecIdentifierInput.setText(mProfile.ipsecIdentifier);
-        mIpsecSecret.setText(mProfile.ipsecSecret);
+        mIpsecSecretInput.setText(mProfile.ipsecSecret);
         final AndroidKeystoreAliasLoader androidKeystoreAliasLoader =
                 new AndroidKeystoreAliasLoader(null);
         loadCertificates(mIpsecUserCert, androidKeystoreAliasLoader.getKeyCertAliases(), 0,
@@ -150,7 +158,8 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
                 R.string.vpn_no_server_cert, mProfile.ipsecServerCert);
         mSaveLogin.setChecked(mProfile.saveLogin);
         mAlwaysOnVpn.setChecked(mProfile.key.equals(VpnUtils.getLockdownVpn()));
-        mPassword.setTextAppearance(android.R.style.TextAppearance_DeviceDefault_Medium);
+        mPasswordInput.getEditText()
+                .setTextAppearance(android.R.style.TextAppearance_DeviceDefault_Medium);
 
         // Hide lockdown VPN on devices that require IMS authentication
         if (SystemProperties.getBoolean("persist.radio.imsregrequired", false)) {
@@ -158,16 +167,16 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
         }
 
         // Third, add listeners to required fields.
-        mName.addTextChangedListener(this);
+        mNameInput.addTextChangedListener(this);
         mType.setOnItemSelectedListener(this);
-        mServer.addTextChangedListener(this);
+        mServerInput.addTextChangedListener(this);
-        mUsername.addTextChangedListener(this);
+        mUsernameInput.addTextChangedListener(this);
-        mPassword.addTextChangedListener(this);
+        mPasswordInput.addTextChangedListener(this);
         mProxySettings.setOnItemSelectedListener(this);
         mProxyHost.addTextChangedListener(this);
         mProxyPort.addTextChangedListener(this);
-        mIpsecIdentifier.addTextChangedListener(this);
+        mIpsecIdentifierInput.addTextChangedListener(this);
-        mIpsecSecret.addTextChangedListener(this);
+        mIpsecSecretInput.addTextChangedListener(this);
         mIpsecUserCert.setOnItemSelectedListener(this);
         mShowOptions.setOnClickListener(this);
         mAlwaysOnVpn.setOnCheckedChangeListener(this);
@@ -202,6 +211,8 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
             setTitle(context.getString(R.string.vpn_connect_to, mProfile.name));
 
             setUsernamePasswordVisibility(mProfile.type);
+            mUsernameInput.setHelperText(context.getString(R.string.vpn_required));
+            mPasswordInput.setHelperText(context.getString(R.string.vpn_required));
 
             // Create a button to connect the network.
             setButton(DialogInterface.BUTTON_POSITIVE,
@@ -260,6 +271,10 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
             updateProxyFieldsVisibility(position);
         }
         updateUiControls();
+        mNameInput.setError("");
+        mServerInput.setError("");
+        mIpsecIdentifierInput.setError("");
+        mIpsecSecretInput.setError("");
     }
 
     @Override
@@ -375,30 +390,16 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
             return false;
         }
 
-        final int position = mType.getSelectedItemPosition();
-        final int type = VPN_TYPES.get(position);
-        if (!editing && requiresUsernamePassword(type)) {
-            return mUsername.getText().length() != 0 && mPassword.getText().length() != 0;
-        }
-        if (mName.getText().length() == 0 || mServer.getText().length() == 0) {
-            return false;
-        }
-
-        // All IKEv2 methods require an identifier
-        if (mIpsecIdentifier.getText().length() == 0) {
-            return false;
-        }
-
         if (!validateProxy()) {
             return false;
         }
 
-        switch (type) {
+        switch (getVpnType()) {
             case VpnProfile.TYPE_IKEV2_IPSEC_USER_PASS:
                 return true;
 
             case VpnProfile.TYPE_IKEV2_IPSEC_PSK:
-                return mIpsecSecret.getText().length() != 0;
+                return true;
 
             case VpnProfile.TYPE_IKEV2_IPSEC_RSA:
                 return mIpsecUserCert.getSelectedItemPosition() != 0;
@@ -406,6 +407,29 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
         return false;
     }
 
+    public boolean validate() {
+        boolean isValidate = true;
+        int type = getVpnType();
+        if (!mEditing && requiresUsernamePassword(type)) {
+            if (!mUsernameInput.validate()) isValidate = false;
+            if (!mPasswordInput.validate()) isValidate = false;
+            return isValidate;
+        }
+
+        if (!mNameInput.validate()) isValidate = false;
+        if (!mServerInput.validate()) isValidate = false;
+        if (!mIpsecIdentifierInput.validate()) isValidate = false;
+        if (type == VpnProfile.TYPE_IKEV2_IPSEC_PSK && !mIpsecSecretInput.validate()) {
+            isValidate = false;
+        }
+        if (!isValidate) Log.w(TAG, "Failed to validate VPN profile!");
+        return isValidate;
+    }
+
+    private int getVpnType() {
+        return VPN_TYPES.get(mType.getSelectedItemPosition());
+    }
+
     private void setTypesByFeature(Spinner typeSpinner) {
         String[] types = getContext().getResources().getStringArray(R.array.vpn_types);
         if (types.length != VPN_TYPES.size()) {
@@ -487,15 +511,14 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
     VpnProfile getProfile() {
         // First, save common fields.
         VpnProfile profile = new VpnProfile(mProfile.key);
-        profile.name = mName.getText().toString();
+        profile.name = mNameInput.getText();
-        final int position = mType.getSelectedItemPosition();
+        profile.type = getVpnType();
-        profile.type = VPN_TYPES.get(position);
+        profile.server = mServerInput.getText().trim();
-        profile.server = mServer.getText().toString().trim();
+        profile.username = mUsernameInput.getText();
-        profile.username = mUsername.getText().toString();
+        profile.password = mPasswordInput.getText();
-        profile.password = mPassword.getText().toString();
 
         // Save fields based on VPN type.
-        profile.ipsecIdentifier = mIpsecIdentifier.getText().toString();
+        profile.ipsecIdentifier = mIpsecIdentifierInput.getText();
 
         if (hasProxy()) {
             String proxyHost = mProxyHost.getText().toString().trim();
@@ -517,7 +540,7 @@ class ConfigDialog extends AlertDialog implements TextWatcher,
         // Then, save type-specific fields.
         switch (profile.type) {
             case VpnProfile.TYPE_IKEV2_IPSEC_PSK:
-                profile.ipsecSecret = mIpsecSecret.getText().toString();
+                profile.ipsecSecret = mIpsecSecretInput.getText();
                 break;
 
             case VpnProfile.TYPE_IKEV2_IPSEC_RSA:

src/com/android/settings/vpn2/ConfigDialogFragment.java

@@ -124,6 +124,7 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
         VpnProfile profile = dialog.getProfile();
 
         if (button == DialogInterface.BUTTON_POSITIVE) {
+            if (!dialog.validate()) return;
             // Possibly throw up a dialog to explain lockdown VPN.
             final boolean shouldLockdown = dialog.isVpnAlwaysOn();
             final boolean shouldConnect = shouldLockdown || !dialog.isEditing();

src/com/android/settings/wifi/WifiConfigController2.java

@@ -77,7 +77,7 @@ import com.android.settings.utils.AndroidKeystoreAliasLoader;
 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.SsidInputGroup;
+import com.android.settings.wifi.utils.TextInputGroup;
 import com.android.settingslib.Utils;
 import com.android.settingslib.utils.ThreadUtils;
 import com.android.wifi.flags.Flags;
@@ -229,7 +229,7 @@ public class WifiConfigController2 implements TextWatcher,
     private final boolean mHideMeteredAndPrivacy;
     private final WifiManager mWifiManager;
     private final AndroidKeystoreAliasLoader mAndroidKeystoreAliasLoader;
-    private SsidInputGroup mSsidInputGroup;
+    private TextInputGroup mSsidInputGroup;
 
     private final Context mContext;
 
@@ -299,7 +299,8 @@ public class WifiConfigController2 implements TextWatcher,
             wepWarningLayout.setVisibility(View.VISIBLE);
         }
 
-        mSsidInputGroup = new SsidInputGroup(mContext, mView, R.id.ssid_layout, R.id.ssid);
+        mSsidInputGroup = new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid,
+                R.string.wifi_ssid_hint);
         mSsidScanButton = (ImageButton) mView.findViewById(R.id.ssid_scanner_button);
         mIpSettingsSpinner = (Spinner) mView.findViewById(R.id.ip_settings);
         mIpSettingsSpinner.setOnItemSelectedListener(this);

src/com/android/settings/wifi/WifiDialog.java

@@ -28,7 +28,7 @@ import android.widget.TextView;
 import androidx.appcompat.app.AlertDialog;
 
 import com.android.settings.R;
-import com.android.settings.wifi.utils.SsidInputGroup;
+import com.android.settings.wifi.utils.TextInputGroup;
 import com.android.settings.wifi.utils.WifiDialogHelper;
 import com.android.settingslib.RestrictedLockUtils;
 import com.android.settingslib.RestrictedLockUtilsInternal;
@@ -120,7 +120,8 @@ public class WifiDialog extends AlertDialog implements WifiConfigUiBase,
         }
 
         mDialogHelper = new WifiDialogHelper(this,
-                new SsidInputGroup(getContext(), mView, R.id.ssid_layout, R.id.ssid));
+                new TextInputGroup(mView, R.id.ssid_layout, R.id.ssid,
+                        R.string.vpn_field_required));
     }
 
     @SuppressWarnings("MissingSuperCall") // TODO: Fix me

src/com/android/settings/wifi/utils/SsidInputGroup.kt

@@ -1,34 +0,0 @@
-/*
- * 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.content.Context
-import android.view.View
-import com.android.settings.R
-
-/** TextInputGroup for Wi-Fi SSID. */
-class SsidInputGroup(private val context: Context, view: View, layoutId: Int, editTextId: Int) :
-    TextInputGroup(view, layoutId, editTextId) {
-
-    fun validate(): Boolean {
-        if (getText().isEmpty()) {
-            setError(context.getString(R.string.wifi_ssid_hint))
-            return false
-        }
-        return true
-    }
-}

src/com/android/settings/wifi/utils/TextInputGroup.kt

@@ -18,6 +18,7 @@ package com.android.settings.wifi.utils
 
 import android.text.Editable
 import android.text.TextWatcher
+import android.util.Log
 import android.view.View
 import android.widget.EditText
 import com.google.android.material.textfield.TextInputLayout
@@ -27,13 +28,17 @@ open class TextInputGroup(
     private val view: View,
     private val layoutId: Int,
     private val editTextId: Int,
+    private val errorMessageId: Int,
 ) {
 
-    private val View.layout: TextInputLayout?
+    val layout: TextInputLayout
-        get() = findViewById(layoutId)
+        get() = view.requireViewById(layoutId)
 
-    private val View.editText: EditText?
+    val editText: EditText
-        get() = findViewById(editTextId)
+        get() = view.requireViewById(editTextId)
+
+    val errorMessage: String
+        get() = view.context.getString(errorMessageId)
 
     private val textWatcher =
         object : TextWatcher {
@@ -42,7 +47,7 @@ open class TextInputGroup(
             override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {}
 
             override fun afterTextChanged(s: Editable?) {
-                view.layout?.isErrorEnabled = false
+                layout.isErrorEnabled = false
             }
         }
 
@@ -51,18 +56,37 @@ open class TextInputGroup(
     }
 
     fun addTextChangedListener(watcher: TextWatcher) {
-        view.editText?.addTextChangedListener(watcher)
+        editText.addTextChangedListener(watcher)
     }
 
-    fun getText(): String {
+    var text: String
-        return view.editText?.text?.toString() ?: ""
+        get() = editText.text?.toString() ?: ""
+        set(value) {
+            editText.setText(value)
         }
 
-    fun setText(text: String) {
+    var helperText: String
-        view.editText?.setText(text)
+        get() = layout.helperText?.toString() ?: ""
+        set(value) {
+            layout.setHelperText(value)
         }
 
-    fun setError(errorMessage: String?) {
+    var error: String
-        view.layout?.apply { error = errorMessage }
+        get() = layout.error?.toString() ?: ""
+        set(value) {
+            layout.setError(value)
+        }
+
+    open fun validate(): Boolean {
+        val isValid = text.isNotEmpty()
+        if (!isValid) {
+            Log.w(TAG, "validate failed in ${layout.hint ?: "unknown"}")
+            error = errorMessage.toString()
+        }
+        return isValid
+    }
+
+    companion object {
+        const val TAG = "TextInputGroup"
     }
 }

src/com/android/settings/wifi/utils/WifiDialogHelper.kt

@@ -21,7 +21,7 @@ import androidx.appcompat.app.AlertDialog
 
 class WifiDialogHelper(
     alertDialog: AlertDialog,
-    private val ssidInputGroup: SsidInputGroup? = null,
+    private val ssidInputGroup: TextInputGroup? = null,
 ) : AlertDialogHelper(alertDialog) {
 
     override fun canDismiss(): Boolean {