Bug 1616468 - Indicate required field in hint string. r=Jamie

Android does not currently have anything similar to a 'required' state
to indicate that a field or input is required before submission. In this
patch we append a localized "required" string onto the node's hint.

The hint typically has the description of the node. If the node is an
entry the hint will have its label followed by the description.

Differential Revision: https://phabricator.services.mozilla.com/D65215

--HG--
extra : moz-landing-system : lando

accessible/android/AccessibleWrap.cpp

@@ -16,11 +16,11 @@
 #include "TextLeafAccessible.h"
 #include "TraversalRule.h"
 #include "Pivot.h"
+#include "Platform.h"
 #include "nsAccessibilityService.h"
 #include "nsEventShell.h"
 #include "nsPersistentProperties.h"
 #include "nsIAccessibleAnnouncementEvent.h"
-#include "nsIStringBundle.h"
 #include "nsAccUtils.h"
 #include "nsTextEquivUtils.h"
 #include "RootAccessible.h"
@@ -28,8 +28,6 @@
 #include "mozilla/a11y/PDocAccessibleChild.h"
 #include "mozilla/jni/GeckoBundleUtils.h"
 
-#define ROLE_STRINGS_URL "chrome://global/locale/AccessFu.properties"
-
 // icu TRUE conflicting with java::sdk::Boolean::TRUE()
 // https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/intl/icu/source/common/unicode/umachine.h#265
 // https://searchfox.org/mozilla-central/source/__GENERATED__/widget/android/bindings/JavaBuiltins.h#78
@@ -535,42 +533,19 @@ void AccessibleWrap::GetRoleDescription(role aRole,
                                         nsIPersistentProperties* aAttributes,
                                         nsAString& aGeckoRole,
                                         nsAString& aRoleDescription) {
-  nsresult rv = NS_OK;
-
-  nsCOMPtr<nsIStringBundleService> sbs =
-      do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv);
-  if (NS_FAILED(rv)) {
-    NS_WARNING("Failed to get string bundle service");
-    return;
-  }
-
-  nsCOMPtr<nsIStringBundle> bundle;
-  rv = sbs->CreateBundle(ROLE_STRINGS_URL, getter_AddRefs(bundle));
-  if (NS_FAILED(rv)) {
-    NS_WARNING("Failed to get string bundle");
-    return;
-  }
-
   if (aRole == roles::HEADING && aAttributes) {
     // The heading level is an attribute, so we need that.
     AutoTArray<nsString, 1> formatString;
-    rv = aAttributes->GetStringProperty(NS_LITERAL_CSTRING("level"),
+    nsresult rv = aAttributes->GetStringProperty(NS_LITERAL_CSTRING("level"),
                                                  *formatString.AppendElement());
-    if (NS_SUCCEEDED(rv)) {
-      rv = bundle->FormatStringFromName("headingLevel", formatString,
-                                        aRoleDescription);
-      if (NS_SUCCEEDED(rv)) {
+    if (NS_SUCCEEDED(rv) &&
+        LocalizeString("headingLevel", aRoleDescription, formatString)) {
       return;
     }
   }
-  }
 
   GetAccService()->GetStringRole(aRole, aGeckoRole);
-  rv = bundle->GetStringFromName(NS_ConvertUTF16toUTF8(aGeckoRole).get(),
-                                 aRoleDescription);
-  if (NS_FAILED(rv)) {
-    aRoleDescription.AssignLiteral("");
-  }
+  LocalizeString(NS_ConvertUTF16toUTF8(aGeckoRole).get(), aRoleDescription);
 }
 
 already_AddRefed<nsIPersistentProperties>
@@ -713,13 +688,10 @@ mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle(
   GECKOBUNDLE_PUT(nodeInfo, "className",
                   java::sdk::Integer::ValueOf(AndroidClass()));
 
+  nsAutoString hint;
   if (aState & states::EDITABLE) {
-    nsAutoString hint(aName);
-    if (!aDescription.IsEmpty()) {
-      hint.AppendLiteral(" ");
-      hint.Append(aDescription);
-    }
-    GECKOBUNDLE_PUT(nodeInfo, "hint", jni::StringParam(hint));
+    // An editable field's name is populated in the hint.
+    hint.Assign(aName);
     GECKOBUNDLE_PUT(nodeInfo, "text", jni::StringParam(aTextValue));
   } else {
     if (role == roles::LINK || role == roles::HEADING) {
@@ -727,10 +699,30 @@ mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle(
     } else {
       GECKOBUNDLE_PUT(nodeInfo, "text", jni::StringParam(aName));
     }
+  }
 
   if (!aDescription.IsEmpty()) {
-      GECKOBUNDLE_PUT(nodeInfo, "hint", jni::StringParam(aDescription));
+    if (!hint.IsEmpty()) {
+      // If this is an editable, the description is concatenated with a
+      // whitespace directly after the name.
+      hint.AppendLiteral(" ");
     }
+    hint.Append(aDescription);
+  }
+
+  if ((aState & states::REQUIRED) != 0) {
+    nsAutoString requiredString;
+    if (LocalizeString("stateRequired", requiredString)) {
+      if (!hint.IsEmpty()) {
+        // If the hint is non-empty, concatenate with a comma for a brief pause.
+        hint.AppendLiteral(", ");
+      }
+      hint.Append(requiredString);
+    }
+  }
+
+  if (!hint.IsEmpty()) {
+    GECKOBUNDLE_PUT(nodeInfo, "hint", jni::StringParam(hint));
   }
 
   nsAutoString geckoRole;

accessible/android/Platform.cpp

@@ -11,13 +11,18 @@
 #include "mozilla/a11y/ProxyAccessible.h"
 #include "nsIAccessibleEvent.h"
 #include "nsIAccessiblePivot.h"
+#include "nsIStringBundle.h"
+
+#define ROLE_STRINGS_URL "chrome://global/locale/AccessFu.properties"
 
 using namespace mozilla;
 using namespace mozilla::a11y;
 
+static nsIStringBundle* sStringBundle;
+
 void a11y::PlatformInit() {}
 
-void a11y::PlatformShutdown() {}
+void a11y::PlatformShutdown() { NS_IF_RELEASE(sStringBundle); }
 
 void a11y::ProxyCreated(ProxyAccessible* aProxy, uint32_t aInterfaces) {
   AccessibleWrap* wrapper = nullptr;
@@ -208,3 +213,43 @@ void a11y::ProxyBatch(ProxyAccessible* aDocument, const uint64_t aBatchType,
       break;
   }
 }
+
+bool a11y::LocalizeString(const char* aToken, nsAString& aLocalized,
+                          const nsTArray<nsString>& aFormatString) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+  nsresult rv = NS_OK;
+  if (!sStringBundle) {
+    nsCOMPtr<nsIStringBundleService> sbs = services::GetStringBundleService();
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Failed to get string bundle service");
+      return false;
+    }
+
+    nsCOMPtr<nsIStringBundle> sb;
+    rv = sbs->CreateBundle(ROLE_STRINGS_URL, getter_AddRefs(sb));
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Failed to get string bundle");
+      return false;
+    }
+
+    sb.forget(&sStringBundle);
+  }
+
+  MOZ_ASSERT(sStringBundle);
+
+  if (aFormatString.Length()) {
+    rv = sStringBundle->FormatStringFromName(aToken, aFormatString, aLocalized);
+    if (NS_SUCCEEDED(rv)) {
+      return true;
+    }
+  } else {
+    rv = sStringBundle->GetStringFromName(aToken, aLocalized);
+    if (NS_SUCCEEDED(rv)) {
+      return true;
+    }
+  }
+
+  NS_WARNING("Failed to localize string");
+  aLocalized.AssignLiteral("");
+  return false;
+}

accessible/base/Platform.h

@@ -131,6 +131,10 @@ class BatchData;
 void ProxyBatch(ProxyAccessible* aDocument, const uint64_t aBatchType,
                 const nsTArray<ProxyAccessible*>& aAccessibles,
                 const nsTArray<BatchData>& aData);
+
+bool LocalizeString(
+    const char* aToken, nsAString& aLocalized,
+    const nsTArray<nsString>& aFormatString = nsTArray<nsString>());
 #endif
 
 }  // namespace a11y

mobile/android/geckoview/src/androidTest/assets/www/accessibility/test-text-entry-node.html

@@ -1,2 +1,3 @@
 <input aria-label='Name' aria-describedby='desc' value='Tobias'>
 <div id='desc'>description</div>
+<input aria-label='Last' value='Funke' required>

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt

@@ -251,7 +251,7 @@ class AccessibilityTest : BaseSessionTest() {
         loadTestPage("test-text-entry-node")
         waitForInitialFocus()
 
-        mainSession.evaluateJS("document.querySelector('input').focus()")
+        mainSession.evaluateJS("document.querySelector('input[aria-label=Name]').focus()")
 
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)
@@ -267,6 +267,23 @@ class AccessibilityTest : BaseSessionTest() {
                 }
             }
         })
+
+        mainSession.evaluateJS("document.querySelector('input[aria-label=Last]').focus()")
+
+        sessionRule.waitUntilCalled(object : EventDelegate {
+            @AssertCalled(count = 1)
+            override fun onFocused(event: AccessibilityEvent) {
+                val nodeId = getSourceId(event)
+                val node = createNodeInfo(nodeId)
+                assertThat("Focused EditBox", node.className.toString(),
+                        equalTo("android.widget.EditText"))
+                if (Build.VERSION.SDK_INT >= 19) {
+                    assertThat("Hint has field name",
+                            node.extras.getString("AccessibilityNodeInfo.hint"),
+                            equalTo("Last, required"))
+                }
+            }
+        })
     }
 
     @Test fun testMoveCaretAccessibilityFocus() {