[Autofill Assistant] Allow accessibility strings to be null

Change the way we handle accessibility strings to that there are three options:

not set -> the default description is used
empty string -> the view is ignored by accessibility
non-empty string -> the string is used as description

Bug: b/143757329
Change-Id: I39cd9bf4a634fa7482c485320d2a81d8513eb935
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1926590
Commit-Queue: Luca Hunkeler <hluca@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#719542}

chrome/android/features/autofill_assistant/BUILD.gn

@@ -67,6 +67,7 @@ android_library("java") {
 
   java_files = [
     "java/src/org/chromium/chrome/browser/autofill_assistant/AbstractListObserver.java",
+    "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAccessibilityUtils.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantBottomBarCoordinator.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantBottomSheetContent.java",
     "java/src/org/chromium/chrome/browser/autofill_assistant/AssistantCoordinator.java",

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantAccessibilityUtils.java

+// Copyright 2019 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.
+
+package org.chromium.chrome.browser.autofill_assistant;
+
+import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_AUTO;
+import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_NO;
+
+import android.support.annotation.Nullable;
+import android.text.TextUtils;
+import android.view.View;
+
+/**
+ * Common accessibility utilities used by autofill assistant.
+ */
+public class AssistantAccessibilityUtils {
+    /*
+     * Sets the importance for accessibility and the description for the given field.
+     * */
+    public static void setAccessibility(View view, @Nullable String accessibilityHint) {
+        view.setContentDescription(accessibilityHint);
+        if (accessibilityHint != null && TextUtils.isEmpty(accessibilityHint)) {
+            view.setImportantForAccessibility(IMPORTANT_FOR_ACCESSIBILITY_NO);
+        } else {
+            view.setImportantForAccessibility(IMPORTANT_FOR_ACCESSIBILITY_AUTO);
+        }
+    }
+}

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/details/AssistantDetailsViewBinder.java

@@ -4,8 +4,7 @@
 
 package org.chromium.chrome.browser.autofill_assistant.details;
 
-import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_AUTO;
-import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_NO;
+import static org.chromium.chrome.browser.autofill_assistant.AssistantAccessibilityUtils.setAccessibility;
 
 import android.animation.Animator;
 import android.animation.AnimatorListenerAdapter;
@@ -157,11 +156,8 @@ class AssistantDetailsViewBinder
                 details.getTotalPrice().isEmpty() ? View.GONE : View.VISIBLE);
 
         viewHolder.mImageView.setVisibility(View.VISIBLE);
-        viewHolder.mImageView.setContentDescription(details.getImageAccessibilityHint());
-        viewHolder.mImageView.setImportantForAccessibility(
-                TextUtils.isEmpty(details.getImageAccessibilityHint())
-                        ? IMPORTANT_FOR_ACCESSIBILITY_NO
-                        : IMPORTANT_FOR_ACCESSIBILITY_AUTO);
+        setAccessibility(viewHolder.mImageView, details.getImageAccessibilityHint());
+
         if (details.getImageUrl().isEmpty()) {
             if (details.getShowImagePlaceholder()) {
                 viewHolder.mImageView.setImageDrawable(viewHolder.mDefaultImage);

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/user_data/AssistantLoginSection.java

@@ -4,8 +4,7 @@
 
 package org.chromium.chrome.browser.autofill_assistant.user_data;
 
-import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_AUTO;
-import static android.view.View.IMPORTANT_FOR_ACCESSIBILITY_NO;
+import static org.chromium.chrome.browser.autofill_assistant.AssistantAccessibilityUtils.setAccessibility;
 
 import android.content.Context;
 import android.support.annotation.Nullable;
@@ -59,11 +58,7 @@ public class AssistantLoginSection extends AssistantCollectUserDataSection<Assis
             sublabelView.setVisibility(View.GONE);
         } else {
             sublabelView.setText(option.getSublabel());
-            sublabelView.setContentDescription(option.getSublabelAccessibilityHint());
-            sublabelView.setImportantForAccessibility(
-                    TextUtils.isEmpty(option.getSublabelAccessibilityHint())
-                            ? IMPORTANT_FOR_ACCESSIBILITY_NO
-                            : IMPORTANT_FOR_ACCESSIBILITY_AUTO);
+            setAccessibility(sublabelView, option.getSublabelAccessibilityHint());
         }
     }
 

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -131,14 +131,19 @@ base::android::ScopedJavaLocalRef<jobject> CreateJavaLoginChoiceList(
           base::android::ConvertUTF8ToJavaString(
               env, login_choice.info_popup->text()));
     }
+    base::android::ScopedJavaLocalRef<jstring> jsublabel_accessibility_hint =
+        nullptr;
+    if (login_choice.sublabel_accessibility_hint.has_value()) {
+      jsublabel_accessibility_hint = base::android::ConvertUTF8ToJavaString(
+          env, login_choice.sublabel_accessibility_hint.value());
+    }
     Java_AssistantCollectUserDataModel_addLoginChoice(
         env, jlist,
         base::android::ConvertUTF8ToJavaString(env, login_choice.identifier),
         base::android::ConvertUTF8ToJavaString(env, login_choice.label),
         base::android::ConvertUTF8ToJavaString(env, login_choice.sublabel),
-        base::android::ConvertUTF8ToJavaString(
-            env, login_choice.sublabel_accessibility_hint),
-        login_choice.preselect_priority, jinfo_popup);
+        jsublabel_accessibility_hint, login_choice.preselect_priority,
+        jinfo_popup);
   }
   return jlist;
 }
@@ -1174,14 +1179,18 @@ void UiControllerAndroid::OnDetailsChanged(const Details* details) {
     Java_AssistantDetailsModel_clearDetails(env, jmodel);
     return;
   }
-
+  auto opt_image_accessibility_hint = details->imageAccessibilityHint();
+  base::android::ScopedJavaLocalRef<jstring> jimage_accessibility_hint =
+      nullptr;
+  if (opt_image_accessibility_hint.has_value()) {
+    jimage_accessibility_hint = base::android::ConvertUTF8ToJavaString(
+        env, opt_image_accessibility_hint.value());
+  }
   auto jdetails = Java_AssistantDetails_create(
       env, base::android::ConvertUTF8ToJavaString(env, details->title()),
       details->titleMaxLines(),
       base::android::ConvertUTF8ToJavaString(env, details->imageUrl()),
-      base::android::ConvertUTF8ToJavaString(env,
-                                             details->imageAccessibilityHint()),
-      details->imageAllowClickthrough(),
+      jimage_accessibility_hint, details->imageAllowClickthrough(),
       base::android::ConvertUTF8ToJavaString(env, details->imageDescription()),
       base::android::ConvertUTF8ToJavaString(env, details->imagePositiveText()),
       base::android::ConvertUTF8ToJavaString(env, details->imageNegativeText()),

components/autofill_assistant/browser/actions/collect_user_data_action.cc

@@ -637,7 +637,10 @@ CollectUserDataAction::CreateOptionsFromProto() {
                 collect_user_data_options->login_choices.size()),
             login_option.custom().label(),
             login_option.sublabel(),
-            login_option.sublabel_accessibility_hint(),
+            login_option.has_sublabel_accessibility_hint()
+                ? base::make_optional(
+                      login_option.sublabel_accessibility_hint())
+                : base::nullopt,
             login_option.has_preselection_priority()
                 ? login_option.preselection_priority()
                 : -1,

components/autofill_assistant/browser/details.cc

@@ -194,7 +194,7 @@ base::Value Details::GetDebugContext() const {
   if (!proto_.image_url().empty())
     dict.SetKey("image_url", base::Value(proto_.image_url()));
 
-  if (!proto_.image_accessibility_hint().empty())
+  if (proto_.has_image_accessibility_hint())
     dict.SetKey("image_accessibility_hint",
                 base::Value(proto_.image_accessibility_hint()));
 
@@ -366,8 +366,11 @@ const std::string Details::imageUrl() const {
   return proto_.image_url();
 }
 
-const std::string Details::imageAccessibilityHint() const {
-  return proto_.image_accessibility_hint();
+const base::Optional<std::string> Details::imageAccessibilityHint() const {
+  if (proto_.has_image_accessibility_hint()) {
+    return proto_.image_accessibility_hint();
+  }
+  return base::nullopt;
 }
 
 bool Details::imageAllowClickthrough() const {

components/autofill_assistant/browser/details.h

@@ -56,7 +56,7 @@ class Details {
   const std::string title() const;
   int titleMaxLines() const;
   const std::string imageUrl() const;
-  const std::string imageAccessibilityHint() const;
+  const base::Optional<std::string> imageAccessibilityHint() const;
   bool imageAllowClickthrough() const;
   const std::string imageDescription() const;
   const std::string imagePositiveText() const;

components/autofill_assistant/browser/user_data.cc

@@ -10,12 +10,13 @@
 
 namespace autofill_assistant {
 
-LoginChoice::LoginChoice(const std::string& _identifier,
-                         const std::string& _label,
-                         const std::string& _sublabel,
-                         const std::string& _sublabel_accessibility_hint,
-                         int _preselect_priority,
-                         const base::Optional<InfoPopupProto>& _info_popup)
+LoginChoice::LoginChoice(
+    const std::string& _identifier,
+    const std::string& _label,
+    const std::string& _sublabel,
+    const base::Optional<std::string>& _sublabel_accessibility_hint,
+    int _preselect_priority,
+    const base::Optional<InfoPopupProto>& _info_popup)
     : identifier(_identifier),
       label(_label),
       sublabel(_sublabel),

components/autofill_assistant/browser/user_data.h

@@ -42,7 +42,7 @@ struct LoginChoice {
   LoginChoice(const std::string& id,
               const std::string& label,
               const std::string& sublabel,
-              const std::string& sublabel_accessibility_hint,
+              const base::Optional<std::string>& sublabel_accessibility_hint,
               int priority,
               const base::Optional<InfoPopupProto>& info_popup);
   LoginChoice(const LoginChoice& another);
@@ -55,7 +55,7 @@ struct LoginChoice {
   // The sublabel to display to the user.
   std::string sublabel;
   // The a11y hint for |sublabel|.
-  std::string sublabel_accessibility_hint;
+  base::Optional<std::string> sublabel_accessibility_hint;
   // The priority to pre-select this choice (-1 == not set/automatic).
   int preselect_priority = -1;
   // The popup to show to provide more information about this login choice.