Chrome OS - Pin Autosubmit - A11Y Improvements

Improve the accessibility behavior of the pin input field used
for PIN auto submit.

Bug: 1075994, 1122675
Change-Id: I68ae534683d5e1bf76e978da8bab6b02f985b758
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364637Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Auto-Submit: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/heads/master@{#803523}

ash/BUILD.gn

@@ -1925,6 +1925,7 @@ test("ash_unittests") {
     "login/ui/login_keyboard_test_base.cc",
     "login/ui/login_keyboard_test_base.h",
     "login/ui/login_password_view_test.cc",
+    "login/ui/login_pin_input_view_unittest.cc",
     "login/ui/login_pin_view_unittest.cc",
     "login/ui/login_public_account_user_view_unittest.cc",
     "login/ui/login_test_base.cc",

ash/ash_strings.grd

@@ -1997,6 +1997,11 @@ This file contains the strings for ash.
       <message name="IDS_ASH_LOGIN_POD_PASSWORD_PIN_INPUT_ACCESSIBLE_NAME" desc="The accessible name of the pin input field used for PIN Auto-submit">
         Enter your PIN
       </message>
+      <message name="IDS_ASH_LOGIN_PIN_INPUT_DIGITS_REMAINING" desc="Screen reader message telling the user how many digits are remaining for the PIN input field. [ICU Syntax]">
+        {NUM_DIGITS, plural,
+         =1 {One digit remaining}
+         other {# digits remaining}}
+      </message>
       <message name="IDS_ASH_LOGIN_SWITCH_TO_PASSWORD" desc="Text to display on the button that is used for switching between PIN and password authentication">
         Switch to password
       </message>

ash/ash_strings_grd/IDS_ASH_LOGIN_PIN_INPUT_DIGITS_REMAINING.png.sha1

+060888bf43ead60dd570bcd522d4454c4d890122
\ No newline at end of file

ash/login/ui/access_code_input.cc

@@ -193,7 +193,8 @@ FixedLengthCodeInput::FixedLengthCodeInput(int length,
                                            bool obscure_pin)
     : on_input_change_(std::move(on_input_change)),
       on_enter_(std::move(on_enter)),
-      on_escape_(std::move(on_escape)) {
+      on_escape_(std::move(on_escape)),
+      is_obscure_pin_(obscure_pin) {
   DCHECK_LT(0, length);
   DCHECK(on_input_change_);
 
@@ -211,7 +212,7 @@ FixedLengthCodeInput::FixedLengthCodeInput(int length,
         gfx::Size(kAccessCodeInputFieldWidthDp, kAccessCodeInputFieldHeightDp));
     field->SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_CENTER);
     field->SetBackgroundColor(SK_ColorTRANSPARENT);
-    if (obscure_pin) {
+    if (is_obscure_pin_) {
       field->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
     } else {
       field->SetTextInputType(ui::TEXT_INPUT_TYPE_NUMBER);
@@ -232,7 +233,7 @@ FixedLengthCodeInput::FixedLengthCodeInput(int length,
     layout->SetFlexForView(field, 1);
   }
 
-  text_value_for_a11y_ = std::string(length, ' ');
+  text_value_for_a11y_ = base::string16(length, ' ');
 }
 
 FixedLengthCodeInput::~FixedLengthCodeInput() = default;
@@ -303,11 +304,16 @@ void FixedLengthCodeInput::RequestFocus() {
 }
 
 void FixedLengthCodeInput::ResetTextValueForA11y() {
-  std::string result = std::string(input_fields_.size(), ' ');
+  base::string16 result;
 
   for (size_t i = 0; i < input_fields_.size(); ++i) {
-    if (!input_fields_[i]->GetText().empty())
-      result[i] = base::UTF16ToUTF8(input_fields_[i]->GetText())[0];
+    if (input_fields_[i]->GetText().empty()) {
+      result.push_back(' ');
+    } else {
+      result.push_back(is_obscure_pin_ ?
+                       base::UTF8ToUTF16("\u2022" /*bullet*/)[0]
+                     : input_fields_[i]->GetText()[0]);
+    }
   }
 
   text_value_for_a11y_ = result;
@@ -333,6 +339,8 @@ void FixedLengthCodeInput::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   const gfx::Range& range = GetSelectedRangeOfTextValueForA11y();
   node_data->AddIntAttribute(ax::mojom::IntAttribute::kTextSelStart,
                              range.start());
+  if (is_obscure_pin_)
+    node_data->AddState(ax::mojom::State::kProtected);
   node_data->AddIntAttribute(ax::mojom::IntAttribute::kTextSelEnd, range.end());
 }
 

ash/login/ui/access_code_input.h

@@ -227,6 +227,8 @@ class FixedLengthCodeInput : public AccessCodeInput {
   // arrows.
   void SetAllowArrowNavigation(bool allowed);
 
+  int active_input_index() { return active_input_index_; }
+
  private:
   // Moves focus to the current input field.
   void FocusActiveField();
@@ -268,11 +270,16 @@ class FixedLengthCodeInput : public AccessCodeInput {
   // Value of current input, associate with AX event. The value will be the
   // concat string of input fields. i.e. [1][2][3][|][][], text_value_for_a11y_
   // = "123   ".
-  std::string text_value_for_a11y_;
+  base::string16 text_value_for_a11y_;
 
   // Whether the user can navigate the input fields with the arrow keys.
   bool arrow_navigation_allowed_ = true;
 
+  // Whether the digits should be rendered as '*' (bullets) instead of digits.
+  // This also affects the ChromeVox behaviour, preventing the digits from
+  // being read out loud.
+  bool is_obscure_pin_ = true;
+
   base::WeakPtrFactory<FixedLengthCodeInput> weak_ptr_factory_{this};
 };
 

ash/login/ui/login_pin_input_view.cc

@@ -52,6 +52,7 @@ class LoginPinInput : public FixedLengthCodeInput {
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
  private:
+  int length_ = 0;
   LoginPinInputView::OnPinSubmit on_submit_;
   LoginPinInputView::OnPinChanged on_changed_;
 
@@ -68,6 +69,7 @@ LoginPinInput::LoginPinInput(int length,
                            /*on_enter*/ base::DoNothing(),
                            /*on_escape*/ base::DoNothing(),
                            /*obscure_pin*/ true),
+      length_(length),
       on_submit_(on_submit),
       on_changed_(on_changed) {
   // Do not allow the user to navigate to other fields. Only insertion
@@ -112,12 +114,24 @@ bool LoginPinInput::HandleGestureEvent(views::Textfield* sender,
 
 void LoginPinInput::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   FixedLengthCodeInput::GetAccessibleNodeData(node_data);
-  node_data->AddStringAttribute(
-      ax::mojom::StringAttribute::kName,
-      l10n_util::GetStringUTF8(
+  const int inserted_digits = active_input_index();
+  const int remaining_digits = length_ - inserted_digits;
+  node_data->SetDescription(l10n_util::GetPluralStringFUTF16(
+      IDS_ASH_LOGIN_PIN_INPUT_DIGITS_REMAINING, remaining_digits));
+  node_data->SetName(l10n_util::GetStringUTF8(
           IDS_ASH_LOGIN_POD_PASSWORD_PIN_INPUT_ACCESSIBLE_NAME));
 }
 
+LoginPinInputView::TestApi::TestApi(LoginPinInputView* view) : view_(view) {
+  DCHECK(view_);
+}
+
+LoginPinInputView::TestApi::~TestApi() = default;
+
+views::View* LoginPinInputView::TestApi::code_input() {
+  return view_->code_input_;
+}
+
 LoginPinInputView::LoginPinInputView() : length_(kDefaultLength) {
   SetLayoutManager(std::make_unique<views::FillLayout>());
 

ash/login/ui/login_pin_input_view.h

@@ -34,6 +34,17 @@ class ASH_EXPORT LoginPinInputView : public views::View {
   using OnPinSubmit = base::RepeatingCallback<void(const base::string16& pin)>;
   using OnPinChanged = base::RepeatingCallback<void(bool is_empty)>;
 
+  class ASH_EXPORT TestApi {
+   public:
+    explicit TestApi(LoginPinInputView* view);
+    ~TestApi();
+
+    views::View* code_input();
+
+   private:
+    LoginPinInputView* const view_;
+  };
+
   LoginPinInputView();
   LoginPinInputView& operator=(const LoginPinInputView&) = delete;
   LoginPinInputView(const LoginPinInputView&) = delete;
@@ -47,9 +58,6 @@ class ASH_EXPORT LoginPinInputView : public views::View {
   // all field are empty. (Drives the visibility of 'Backspace' on the pin pad.)
   void Init(const OnPinSubmit& on_submit, const OnPinChanged& on_changed);
 
-  // The code input will call this when all digits are in.
-  void SubmitPin(const base::string16& pin);
-
   // Updates the length of the field. Used when switching users.
   void UpdateLength(const size_t pin_length);
 
@@ -66,6 +74,9 @@ class ASH_EXPORT LoginPinInputView : public views::View {
   void RequestFocus() override;
 
  private:
+  // The code input will call this when all digits are in.
+  void SubmitPin(const base::string16& pin);
+
   // Called by the inner view whenever the fields change.
   void OnChanged(bool is_empty);
 

ash/login/ui/login_pin_input_view_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/login/ui/login_pin_input_view.h"
+#include <memory>
+#include <string>
+#include "ash/login/ui/login_test_base.h"
+#include "base/bind.h"
+#include "base/optional.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string16.h"
+#include "ui/accessibility/ax_enums.mojom-shared.h"
+#include "ui/accessibility/ax_node_data.h"
+#include "ui/views/widget/widget.h"
+
+namespace ash {
+
+class LoginPinInputViewTest
+    : public LoginTestBase,
+      public ::testing::WithParamInterface<int> /* length */ {
+ public:
+  static std::string ParamInfoToString(
+      testing::TestParamInfo<LoginPinInputViewTest::ParamType> info) {
+    return base::StrCat({"Length", base::NumberToString(info.param)});
+  }
+
+ protected:
+  LoginPinInputViewTest() = default;
+  ~LoginPinInputViewTest() override = default;
+
+  void SetUp() override {
+    LoginTestBase::SetUp();
+    view_ = new LoginPinInputView();
+    view_->Init(base::BindRepeating(&LoginPinInputViewTest::OnPinSubmit,
+                                    base::Unretained(this)),
+                base::BindRepeating(&LoginPinInputViewTest::OnPinChanged,
+                                    base::Unretained(this)));
+
+    length_ = GetParam();
+    view_->UpdateLength(length_);
+    SetWidget(CreateWidgetWithContent(view_));
+  }
+
+  void OnPinSubmit(const base::string16& pin) {
+    submitted_pin_ = base::make_optional(pin);
+  }
+
+  void OnPinChanged(const bool is_empty) {
+    is_empty_ = base::make_optional(is_empty);
+  }
+
+  void PressKeyHelper(ui::KeyboardCode key) {
+    GetEventGenerator()->PressKey(key, ui::EF_NONE);
+    // Wait until the keypress is processed.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void ExpectAttribute(const std::string& value,
+                       ax::mojom::StringAttribute attribute) {
+    LoginPinInputView::TestApi test_api(view_);
+    ui::AXNodeData ax_node_data;
+    test_api.code_input()->GetAccessibleNodeData(&ax_node_data);
+    EXPECT_EQ(value, ax_node_data.GetStringAttribute(attribute));
+  }
+
+  void ExpectDescription(const std::string& value) {
+    ExpectAttribute(value, ax::mojom::StringAttribute::kDescription);
+  }
+
+  void ExpectTextValue(const std::string& value) {
+    ExpectAttribute(value, ax::mojom::StringAttribute::kValue);
+  }
+
+  LoginPinInputView* view_ = nullptr;
+  int length_ = 0;
+
+  // Generated during the callback response.
+  base::Optional<base::string16> submitted_pin_;
+  base::Optional<bool> is_empty_;
+};
+
+// Tests that ChromeVox announces "Enter your PIN" when the
+// field gets focused
+TEST_P(LoginPinInputViewTest, AccessibleName) {
+  ExpectAttribute("Enter your PIN", ax::mojom::StringAttribute::kName);
+}
+
+// Tests that ChromeVox announces "X digits remaining" when the
+// field gets focused
+TEST_P(LoginPinInputViewTest, AccessibleValues) {
+  ExpectDescription("6 digits remaining");
+  ExpectTextValue("      ");
+
+  PressKeyHelper(ui::KeyboardCode::VKEY_1);
+  ExpectDescription("5 digits remaining");
+  ExpectTextValue("\u2022     ");                     /* 1 bullet 5 spaces */
+
+  PressKeyHelper(ui::KeyboardCode::VKEY_1);
+  ExpectDescription("4 digits remaining");
+  ExpectTextValue("\u2022\u2022    ");                /* 2 bullets 4 spaces */
+
+  PressKeyHelper(ui::KeyboardCode::VKEY_1);
+  ExpectDescription("3 digits remaining");
+  ExpectTextValue("\u2022\u2022\u2022   ");           /* 3 bullets 3 spaces */
+
+  PressKeyHelper(ui::KeyboardCode::VKEY_1);
+  ExpectTextValue("\u2022\u2022\u2022\u2022  ");      /* 4 bullets 2 spaces */
+  ExpectDescription("2 digits remaining");
+
+  PressKeyHelper(ui::KeyboardCode::VKEY_1);
+  ExpectTextValue("\u2022\u2022\u2022\u2022\u2022 "); /* 5 bullets 1 space */
+  ExpectDescription("One digit remaining");
+}
+
+INSTANTIATE_TEST_SUITE_P(PinInputViewTests,
+                         LoginPinInputViewTest,
+                         testing::Values(6),
+                         LoginPinInputViewTest::ParamInfoToString);
+
+}  // namespace ash

ash/login/ui/pin_request_view_unittest.cc

@@ -88,6 +88,7 @@ class PinRequestViewTest : public LoginTestBase,
   void StartView(base::Optional<int> pin_length = 6) {
     PinRequest request;
     request.help_button_enabled = true;
+    request.obscure_pin = false;
     request.pin_length = pin_length;
     request.on_pin_request_done = base::DoNothing::Once<bool>();
     view_ = new PinRequestView(std::move(request), this);