CrOS Captive Portal: Add additional a11y for the 2022 UI

BUG=b:244353766
TEST=ash_unittests

Skip-Translation-Screenshots-Check: True
Change-Id: Id9cdaae7c3195330c93826141f9b3205d59ae451
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3950068
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Auto-Submit: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1058894}

ash/ash_strings.grd

@@ -2222,6 +2222,9 @@ Connect your device to power.
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_A11Y_LABEL_ACTIVATE" desc="Accessibility label used for an unactivated cellular network in quick settings network list that opens the activation flow when clicked.">
         Click to activate <ph name="NETWORK_NAME">$1<ex>Verizon</ex></ph>
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_A11Y_LABEL_SUBTEXT" desc="Accessibility label used for a network in quick settings network list that has a subtext message (e.g. when offline signin is required).">
+        <ph name="NETWORK_NAME">$1<ex>CaptivePortalNetwork</ex></ph>, <ph name="SUBTEXT">$2<ex>Sign in to network</ex></ph>
+      </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_DISCONNECT_BUTTON_A11Y_LABEL" desc="Accessibility label used for a button to disconnect from a network in quick settings network list.">
         Disconnect from <ph name="NETWORK_NAME">$1<ex>Ethernet</ex></ph>
       </message>
@@ -2410,11 +2413,14 @@ Connect your device to power.
         Connected to <ph name="NAME">$1<ex>GoogleGuest</ex></ph>
       </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_TOOLTIP" desc="The tooltip text used when connected to a network.">
-        Connected to <ph name="NAME">$1<ex>GoogleGuest</ex></ph>, <ph name="STRENGTH">$2<ex>Weak signal.</ex></ph>
+        <ph name="DESC">$1<ex>Connected to GoogleGuest</ex></ph>, <ph name="STRENGTH">$2<ex>Weak signal.</ex></ph>
       </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_ACTIVATING" desc="Message for the network tray tooltip and a11y name when activating a network.">
         Activating <ph name="NAME">$1<ex>YBH Cellular</ex></ph>
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_PORTAL" desc="Message for the network tray tooltip and a11y name when connected to a network in a captive portal state.">
+        Connected to <ph name="NAME">$1<ex>GoogleGuest</ex></ph>, <ph name="SUBTEXT">$2<ex>Sign in to network</ex></ph>
+      </message>
 
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_TOGGLE_TOOLTIP" desc="The tooltip text for the button that toggles network enabled/disabled state.">
         Toggle network connection. <ph name="STATE_TEXT">$1<ex>Connected to public wifi</ex></ph>

ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_TOOLTIP.png.sha1

@@ -0,0 +1 @@
+d741dfe72d2c90c6ea7303db5d1060dfcf4931ed

ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_NETWORK_PORTAL.png.sha1

@@ -0,0 +1 @@
+c0f1e085d614307c6fa869550ce4d0e86905ac46

ash/system/network/active_network_icon.cc

@@ -10,6 +10,7 @@
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/model/system_tray_model.h"
 #include "ash/system/network/network_icon.h"
+#include "ash/system/network/network_utils.h"
 #include "ash/system/network/tray_network_state_model.h"
 #include "ash/system/tray/tray_constants.h"
 #include "base/bind.h"
@@ -88,8 +89,14 @@ void ActiveNetworkIcon::GetConnectionStatusStrings(Type type,
       *tooltip = activating_string;
   } else if (network && chromeos::network_config::StateIsConnected(
                             network->connection_state)) {
-    std::u16string connected_string = l10n_util::GetStringFUTF16(
-        IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED, network_name);
+    std::u16string connected_string;
+    if (auto portal_subtext = GetPortalStateSubtext(network->portal_state)) {
+      connected_string = l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_PORTAL, network_name, *portal_subtext);
+    } else {
+      connected_string = l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED, network_name);
+    }
     std::u16string signal_strength_string;
     if (chromeos::network_config::NetworkTypeMatchesType(
             network->type, NetworkType::kWireless)) {
@@ -123,7 +130,7 @@ void ActiveNetworkIcon::GetConnectionStatusStrings(Type type,
                      ? connected_string
                      : l10n_util::GetStringFUTF16(
                            IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_TOOLTIP,
-                           network_name, signal_strength_string);
+                           connected_string, signal_strength_string);
     }
   } else if (network &&
              network->connection_state == ConnectionStateType::kConnecting) {

ash/system/network/active_network_icon_unittest.cc

@@ -190,9 +190,11 @@ TEST_F(ActiveNetworkIconTest, GetConnectionStatusStrings) {
   EXPECT_EQ(l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED,
                                        kCellularNetworkGuid16),
             name);
+  std::u16string connected_string = l10n_util::GetStringFUTF16(
+      IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED, kCellularNetworkGuid16);
   EXPECT_EQ(
       l10n_util::GetStringFUTF16(
-          IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_TOOLTIP, kCellularNetworkGuid16,
+          IDS_ASH_STATUS_TRAY_NETWORK_CONNECTED_TOOLTIP, connected_string,
           l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_STRONG)),
       tooltip);
 }

ash/system/network/network_list_network_item_view.cc

@@ -14,8 +14,10 @@
 #include "ash/style/ash_color_provider.h"
 #include "ash/style/color_util.h"
 #include "ash/system/model/system_tray_model.h"
+#include "ash/system/network/active_network_icon.h"
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_icon_animation.h"
+#include "ash/system/network/network_utils.h"
 #include "ash/system/network/tray_network_state_model.h"
 #include "ash/system/power/power_status.h"
 #include "ash/system/tray/hover_highlight_view.h"
@@ -25,6 +27,7 @@
 #include "base/i18n/number_formatting.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/gfx/image/image_skia.h"
@@ -353,30 +356,16 @@ void NetworkListNetworkItemView::SetupNetworkSubtext() {
     return;
   }
 
-  if (!ash::features::IsCaptivePortalUI2022Enabled()) {
-    SetupConnectedScrollListItem(this);
-    return;
+  if (ash::features::IsCaptivePortalUI2022Enabled()) {
+    absl::optional<std::u16string> portal_subtext =
+        GetPortalStateSubtext(network_properties()->portal_state);
+    if (portal_subtext) {
+      SetWarningSubText(this, *portal_subtext);
+      return;
+    }
   }
 
-  switch (network_properties()->portal_state) {
-    // Portal state is portal or proxy auth, setup signin subtext.
-    case PortalState::kPortal:
-    case PortalState::kProxyAuthRequired:
-      SetWarningSubText(this, l10n_util::GetStringUTF16(
-                                  IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN));
-      return;
-    // Portal state is portal-suspected or no internet, setup no internet
-    // subtext.
-    case PortalState::kPortalSuspected:
-    case PortalState::kNoInternet:
-      SetWarningSubText(
-          this, l10n_util::GetStringUTF16(
-                    IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET));
-      return;
-    default:
-      SetupConnectedScrollListItem(this);
-      return;
-  }
+  SetupConnectedScrollListItem(this);
 }
 
 void NetworkListNetworkItemView::UpdateDisabledTextColor() {
@@ -426,6 +415,13 @@ void NetworkListNetworkItemView::AddPolicyView() {
 
 std::u16string NetworkListNetworkItemView::GenerateAccessibilityLabel(
     const std::u16string& label) {
+  absl::optional<std::u16string> portal_subtext =
+      GetPortalStateSubtext(network_properties()->portal_state);
+  if (portal_subtext) {
+    return l10n_util::GetStringFUTF16(
+        IDS_ASH_STATUS_TRAY_NETWORK_A11Y_LABEL_SUBTEXT, label, *portal_subtext);
+  }
+
   if (IsNetworkConnectable(network_properties())) {
     return l10n_util::GetStringFUTF16(
         IDS_ASH_STATUS_TRAY_NETWORK_A11Y_LABEL_CONNECT, label);
@@ -449,8 +445,14 @@ std::u16string NetworkListNetworkItemView::GenerateAccessibilityDescription() {
   std::u16string connection_status;
 
   if (StateIsConnected(network_properties()->connection_state)) {
-    connection_status =
-        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
+    absl::optional<std::u16string> portal_subtext =
+        GetPortalStateSubtext(network_properties()->portal_state);
+    if (portal_subtext) {
+      connection_status = *portal_subtext;
+    } else {
+      connection_status = l10n_util::GetStringUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
+    }
   } else if (network_properties()->connection_state ==
              ConnectionStateType::kConnecting) {
     connection_status = l10n_util::GetStringUTF16(
@@ -586,4 +588,4 @@ NetworkListNetworkItemView::GenerateAccessibilityDescriptionForTether(
 BEGIN_METADATA(NetworkListNetworkItemView, NetworkListItemView)
 END_METADATA
 
-}  // namespace ash
+}  // namespace ash

ash/system/network/network_list_network_item_view_unittest.cc

@@ -107,6 +107,10 @@ class NetworkListNetworkItemViewTest : public AshTestBase {
     return {OncSource::kDevicePolicy, OncSource::kNone};
   }
 
+  std::vector<PortalState> GetPortalStates() {
+    return {PortalState::kPortal, PortalState::kNoInternet};
+  }
+
   const NetworkListItemView* LastClickedNetworkListItem() {
     return fake_network_detailed_network_view_
         ->last_clicked_network_list_item();
@@ -125,7 +129,7 @@ class NetworkListNetworkItemViewTest : public AshTestBase {
   }
 
   void AssertA11yDescription(NetworkStatePropertiesPtr& network_properties,
-                             const std::u16string& description) {
+                             const std::u16string& expected_description) {
     ui::AXNodeData node_data;
     UpdateViewForNetwork(network_properties);
     network_list_network_item_view()
@@ -133,7 +137,7 @@ class NetworkListNetworkItemViewTest : public AshTestBase {
         .GetAccessibleNodeData(&node_data);
     std::string a11ydescription =
         node_data.GetStringAttribute(ax::mojom::StringAttribute::kDescription);
-    EXPECT_EQ(base::UTF8ToUTF16(a11ydescription), description);
+    EXPECT_EQ(base::UTF8ToUTF16(a11ydescription), expected_description);
   }
 
   void NetworkIconChanged() {
@@ -757,6 +761,111 @@ TEST_F(NetworkListNetworkItemViewTest, HasExpectedDescriptionForWiFi) {
   }
 }
 
+TEST_F(NetworkListNetworkItemViewTest, HasExpectedDescriptionForWiFiWithFlag) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+
+  SecurityType security_types[2] = {SecurityType::kNone, SecurityType::kWepPsk};
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kConnected);
+
+  for (const auto& security : security_types) {
+    wifi_network->type_state->get_wifi()->security = security;
+    const std::u16string security_label = l10n_util::GetStringUTF16(
+        security == SecurityType::kWepPsk
+            ? IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SECURED
+            : IDS_ASH_STATUS_TRAY_NETWORK_STATUS_UNSECURED);
+
+    for (const auto& connection : GetConnectionStateTypes()) {
+      wifi_network->connection_state = connection;
+      wifi_network->portal_state = PortalState::kUnknown;  // default
+      std::u16string connection_status;
+      int desc_id;
+      for (const auto& policy : GetPolicies()) {
+        wifi_network->source = policy;
+        // Set desc_id for portal, online, connecting
+        switch (policy) {
+          case OncSource::kDevicePolicy:
+            desc_id =
+                IDS_ASH_STATUS_TRAY_WIFI_NETWORK_A11Y_DESC_MANAGED_WITH_CONNECTION_STATUS;
+            break;
+          case OncSource::kNone:
+            desc_id =
+                IDS_ASH_STATUS_TRAY_WIFI_NETWORK_A11Y_DESC_WITH_CONNECTION_STATUS;
+            break;
+          default:
+            NOTREACHED();
+        }
+        switch (connection) {
+          case ConnectionStateType::kPortal: {
+            for (const auto& portal_state : GetPortalStates()) {
+              wifi_network->portal_state = portal_state;
+              switch (portal_state) {
+                case PortalState::kPortal:
+                  connection_status = l10n_util::GetStringUTF16(
+                      IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN);
+                  break;
+                case PortalState::kNoInternet:
+                  connection_status = l10n_util::GetStringUTF16(
+                      IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET);
+                  break;
+                default:
+                  NOTREACHED();
+              }
+              AssertA11yDescription(
+                  wifi_network, l10n_util::GetStringFUTF16(
+                                    desc_id, security_label, connection_status,
+                                    base::FormatPercent(kSignalStrength)));
+            }
+            break;
+          }
+          case ConnectionStateType::kConnected:
+            [[fallthrough]];
+          case ConnectionStateType::kOnline: {
+            connection_status = l10n_util::GetStringUTF16(
+                IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED);
+            AssertA11yDescription(
+                wifi_network, l10n_util::GetStringFUTF16(
+                                  desc_id, security_label, connection_status,
+                                  base::FormatPercent(kSignalStrength)));
+            break;
+          }
+          case ConnectionStateType::kConnecting: {
+            connection_status = l10n_util::GetStringUTF16(
+                IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING);
+            AssertA11yDescription(
+                wifi_network, l10n_util::GetStringFUTF16(
+                                  desc_id, security_label, connection_status,
+                                  base::FormatPercent(kSignalStrength)));
+            break;
+          }
+          case ConnectionStateType::kNotConnected: {
+            switch (policy) {
+              case OncSource::kDevicePolicy:
+                desc_id = IDS_ASH_STATUS_TRAY_WIFI_NETWORK_A11Y_DESC_MANAGED;
+                break;
+              case OncSource::kNone:
+                desc_id = IDS_ASH_STATUS_TRAY_WIFI_NETWORK_A11Y_DESC;
+                break;
+              default:
+                NOTREACHED();
+            }
+            AssertA11yDescription(wifi_network,
+                                  l10n_util::GetStringFUTF16(
+                                      desc_id, security_label,
+                                      base::FormatPercent(kSignalStrength)));
+            break;
+          }
+        }
+      }
+    }
+  }
+}
+
 TEST_F(NetworkListNetworkItemViewTest, NetworkIconAnimating) {
   NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
       kWiFiName, NetworkType::kWiFi, ConnectionStateType::kConnecting);

ash/system/network/network_utils.cc

@@ -4,8 +4,11 @@
 
 #include "ash/system/network/network_utils.h"
 
+#include "ash/constants/ash_features.h"
+#include "ash/strings/grit/ash_strings.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/strings/strcat.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace ash {
 
@@ -54,4 +57,30 @@ void RecordNetworkTypeToggled(
       new_state);
 }
 
-}  // namespace ash
+absl::optional<std::u16string> GetPortalStateSubtext(
+    const chromeos::network_config::mojom::PortalState& portal_state) {
+  if (!ash::features::IsCaptivePortalUI2022Enabled()) {
+    return absl::nullopt;
+  }
+  using chromeos::network_config::mojom::PortalState;
+  switch (portal_state) {
+    case PortalState::kUnknown:
+      [[fallthrough]];
+    case PortalState::kOnline:
+      return absl::nullopt;
+    case PortalState::kPortalSuspected:
+      [[fallthrough]];
+    case PortalState::kNoInternet:
+      // Use 'no internet' for portal suspected and no internet states.
+      return l10n_util::GetStringUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET);
+    case PortalState::kPortal:
+      [[fallthrough]];
+    case PortalState::kProxyAuthRequired:
+      // Use 'signin to network' for portal and proxy auth required states.
+      return l10n_util::GetStringUTF16(
+          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN);
+  }
+}
+
+}  // namespace ash

ash/system/network/network_utils.h

@@ -7,6 +7,7 @@
 
 #include "ash/ash_export.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace ash {
 
@@ -40,6 +41,11 @@ ASH_EXPORT void RecordNetworkTypeToggled(
     chromeos::network_config::mojom::NetworkType network_type,
     bool new_state);
 
+// Returns the subtext to display for a connected network in a portal state.
+// This is used in the network menu, the tooltip, and for a11y.
+ASH_EXPORT absl::optional<std::u16string> GetPortalStateSubtext(
+    const chromeos::network_config::mojom::PortalState& portal_state);
+
 }  // namespace ash
 
-#endif  // ASH_SYSTEM_NETWORK_NETWORK_UTILS_H_
+#endif  // ASH_SYSTEM_NETWORK_NETWORK_UTILS_H_