Bug 1510791 - Add action count to BatchData and set isClickable correctly in Android. r=yzen
authorEitan Isaacson <eitan@monotonous.org>
Wed, 05 Dec 2018 15:37:24 +0000
changeset 508633 1d3c0e917ed7306012e6d7408111067a6e2c6393
parent 508632 134001f7ca7fb0a6cfd32d56ea1a324d31fb62e1
child 508634 140bc054964e43cce17d3011b82eca847425642b
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1510791
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1510791 - Add action count to BatchData and set isClickable correctly in Android. r=yzen Differential Revision: https://phabricator.services.mozilla.com/D13320
accessible/android/AccessibleWrap.cpp
accessible/android/AccessibleWrap.h
accessible/android/DocAccessibleWrap.cpp
accessible/android/ProxyAccessibleWrap.cpp
accessible/android/ProxyAccessibleWrap.h
accessible/android/SessionAccessibility.cpp
accessible/ipc/other/PDocAccessible.ipdl
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
--- a/accessible/android/AccessibleWrap.cpp
+++ b/accessible/android/AccessibleWrap.cpp
@@ -209,17 +209,18 @@ bool AccessibleWrap::GetSelectionBounds(
                                         int32_t* aEndOffset) {
   if (IsHyperText()) {
     return AsHyperText()->SelectionBoundsAt(0, aStartOffset, aEndOffset);
   }
 
   return false;
 }
 
-uint32_t AccessibleWrap::GetFlags(role aRole, uint64_t aState) {
+uint32_t AccessibleWrap::GetFlags(role aRole, uint64_t aState,
+                                  uint8_t aActionCount) {
   uint32_t flags = 0;
   if (aState & states::CHECKABLE) {
     flags |= java::SessionAccessibility::FLAG_CHECKABLE;
   }
 
   if (aState & states::CHECKED) {
     flags |= java::SessionAccessibility::FLAG_CHECKED;
   }
@@ -227,17 +228,17 @@ uint32_t AccessibleWrap::GetFlags(role a
   if (aState & states::INVALID) {
     flags |= java::SessionAccessibility::FLAG_CONTENT_INVALID;
   }
 
   if (aState & states::EDITABLE) {
     flags |= java::SessionAccessibility::FLAG_EDITABLE;
   }
 
-  if (aState & states::SENSITIVE) {
+  if (aActionCount && aRole != roles::TEXT_LEAF) {
     flags |= java::SessionAccessibility::FLAG_CLICKABLE;
   }
 
   if (aState & states::ENABLED) {
     flags |= java::SessionAccessibility::FLAG_ENABLED;
   }
 
   if (aState & states::FOCUSABLE) {
@@ -391,39 +392,40 @@ mozilla::java::GeckoBundle::LocalRef Acc
   double curValue = UnspecifiedNaN<double>();
   double minValue = UnspecifiedNaN<double>();
   double maxValue = UnspecifiedNaN<double>();
   double step = UnspecifiedNaN<double>();
   WrapperRangeInfo(&curValue, &minValue, &maxValue, &step);
 
   nsCOMPtr<nsIPersistentProperties> attributes = Attributes();
 
-  return ToBundle(State(), Bounds(), name, textValue, nodeID, curValue,
-                  minValue, maxValue, step, attributes);
+  return ToBundle(State(), Bounds(), ActionCount(), name, textValue, nodeID,
+                  curValue, minValue, maxValue, step, attributes);
 }
 
 mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle(
-    const uint64_t aState, const nsIntRect& aBounds, const nsString& aName,
-    const nsString& aTextValue, const nsString& aDOMNodeID,
-    const double& aCurVal, const double& aMinVal, const double& aMaxVal,
-    const double& aStep, nsIPersistentProperties* aAttributes) {
+    const uint64_t aState, const nsIntRect& aBounds, const uint8_t aActionCount,
+    const nsString& aName, const nsString& aTextValue,
+    const nsString& aDOMNodeID, const double& aCurVal, const double& aMinVal,
+    const double& aMaxVal, const double& aStep,
+    nsIPersistentProperties* aAttributes) {
   if (!IsProxy() && IsDefunct()) {
     return nullptr;
   }
 
   GECKOBUNDLE_START(nodeInfo);
   GECKOBUNDLE_PUT(nodeInfo, "id", java::sdk::Integer::ValueOf(VirtualViewID()));
 
   AccessibleWrap* parent = WrapperParent();
   GECKOBUNDLE_PUT(
       nodeInfo, "parentId",
       java::sdk::Integer::ValueOf(parent ? parent->VirtualViewID() : 0));
 
   role role = WrapperRole();
-  uint32_t flags = GetFlags(role, aState);
+  uint32_t flags = GetFlags(role, aState, aActionCount);
   GECKOBUNDLE_PUT(nodeInfo, "flags", java::sdk::Integer::ValueOf(flags));
   GECKOBUNDLE_PUT(nodeInfo, "className",
                   java::sdk::Integer::ValueOf(AndroidClass()));
 
   if (aState & states::EDITABLE) {
     GECKOBUNDLE_PUT(nodeInfo, "hint", jni::StringParam(aName));
     GECKOBUNDLE_PUT(nodeInfo, "text", jni::StringParam(aTextValue));
   } else {
@@ -551,30 +553,31 @@ mozilla::java::GeckoBundle::LocalRef Acc
   GECKOBUNDLE_PUT(nodeInfo, "children",
                   jni::IntArray::New(children.Elements(), children.Length()));
   GECKOBUNDLE_FINISH(nodeInfo);
 
   return nodeInfo;
 }
 
 mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToSmallBundle() {
-  return ToSmallBundle(State(), Bounds());
+  return ToSmallBundle(State(), Bounds(), ActionCount());
 }
 
 mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToSmallBundle(
-    const uint64_t aState, const nsIntRect& aBounds) {
+    const uint64_t aState, const nsIntRect& aBounds,
+    const uint8_t aActionCount) {
   GECKOBUNDLE_START(nodeInfo);
   GECKOBUNDLE_PUT(nodeInfo, "id", java::sdk::Integer::ValueOf(VirtualViewID()));
 
   AccessibleWrap* parent = WrapperParent();
   GECKOBUNDLE_PUT(
       nodeInfo, "parentId",
       java::sdk::Integer::ValueOf(parent ? parent->VirtualViewID() : 0));
 
-  uint32_t flags = GetFlags(WrapperRole(), aState);
+  uint32_t flags = GetFlags(WrapperRole(), aState, aActionCount);
   GECKOBUNDLE_PUT(nodeInfo, "flags", java::sdk::Integer::ValueOf(flags));
   GECKOBUNDLE_PUT(nodeInfo, "className",
                   java::sdk::Integer::ValueOf(AndroidClass()));
 
   const int32_t data[4] = {aBounds.x, aBounds.y, aBounds.x + aBounds.width,
                            aBounds.y + aBounds.height};
   GECKOBUNDLE_PUT(nodeInfo, "bounds", jni::IntArray::New(data, 4));
 
--- a/accessible/android/AccessibleWrap.h
+++ b/accessible/android/AccessibleWrap.h
@@ -28,23 +28,25 @@ class AccessibleWrap : public Accessible
 
   virtual void GetTextContents(nsAString& aText);
 
   virtual bool GetSelectionBounds(int32_t* aStartOffset, int32_t* aEndOffset);
 
   mozilla::java::GeckoBundle::LocalRef ToBundle();
 
   mozilla::java::GeckoBundle::LocalRef ToBundle(
-      const uint64_t aState, const nsIntRect& aBounds, const nsString& aName,
+      const uint64_t aState, const nsIntRect& aBounds,
+      const uint8_t aActionCount, const nsString& aName,
       const nsString& aTextValue, const nsString& aDOMNodeID,
       const double& aCurVal, const double& aMinVal, const double& aMaxVal,
       const double& aStep, nsIPersistentProperties* aAttributes);
 
-  mozilla::java::GeckoBundle::LocalRef ToSmallBundle(const uint64_t aState,
-                                                     const nsIntRect& aBounds);
+  mozilla::java::GeckoBundle::LocalRef ToSmallBundle(
+      const uint64_t aState, const nsIntRect& aBounds,
+      const uint8_t aActionCount);
 
   mozilla::java::GeckoBundle::LocalRef ToSmallBundle();
 
   virtual void WrapperDOMNodeID(nsString& aDOMNodeID);
 
   int32_t AndroidClass() {
     return mID == kNoID ? java::SessionAccessibility::CLASSNAME_WEBVIEW
                         : GetAndroidClass(WrapperRole());
@@ -73,17 +75,17 @@ class AccessibleWrap : public Accessible
 
   virtual bool WrapperRangeInfo(double* aCurVal, double* aMinVal,
                                 double* aMaxVal, double* aStep);
 
   virtual role WrapperRole() { return Role(); }
 
   static void GetRoleDescription(role aRole, nsAString& aGeckoRole,
                                  nsAString& aRoleDescription);
-  static uint32_t GetFlags(role aRole, uint64_t aState);
+  static uint32_t GetFlags(role aRole, uint64_t aState, uint8_t aActionCount);
 };
 
 static inline AccessibleWrap* WrapperFor(const ProxyAccessible* aProxy) {
   return reinterpret_cast<AccessibleWrap*>(aProxy->GetWrapper());
 }
 
 }  // namespace a11y
 }  // namespace mozilla
--- a/accessible/android/DocAccessibleWrap.cpp
+++ b/accessible/android/DocAccessibleWrap.cpp
@@ -129,20 +129,20 @@ void DocAccessibleWrap::CacheViewportCal
     nsTArray<BatchData> cacheData(inViewAccs.Count());
     for (auto iter = inViewAccs.Iter(); !iter.Done(); iter.Next()) {
       Accessible* accessible = iter.Data();
       auto uid = accessible->IsDoc() && accessible->AsDoc()->IPCDoc()
                      ? 0
                      : reinterpret_cast<uint64_t>(accessible->UniqueID());
       cacheData.AppendElement(
           BatchData(accessible->Document()->IPCDoc(), uid, accessible->State(),
-                    accessible->Bounds(), nsString(), nsString(), nsString(),
+                    accessible->Bounds(), accessible->ActionCount(), nsString(),
+                    nsString(), nsString(), UnspecifiedNaN<double>(),
                     UnspecifiedNaN<double>(), UnspecifiedNaN<double>(),
-                    UnspecifiedNaN<double>(), UnspecifiedNaN<double>(),
-                    nsTArray<Attribute>()));
+                    UnspecifiedNaN<double>(), nsTArray<Attribute>()));
     }
 
     ipcDoc->SendBatch(eBatch_Viewport, cacheData);
   } else if (SessionAccessibility* sessionAcc =
                  SessionAccessibility::GetInstanceFor(docAcc)) {
     nsTArray<AccessibleWrap*> accessibles(inViewAccs.Count());
     for (auto iter = inViewAccs.Iter(); !iter.Done(); iter.Next()) {
       accessibles.AppendElement(
@@ -194,20 +194,20 @@ void DocAccessibleWrap::CacheFocusPath(A
       acc->Name(name);
       nsAutoString textValue;
       acc->Value(textValue);
       nsAutoString nodeID;
       acc->WrapperDOMNodeID(nodeID);
       nsCOMPtr<nsIPersistentProperties> props = acc->Attributes();
       nsTArray<Attribute> attributes;
       nsAccUtils::PersistentPropertiesToArray(props, &attributes);
-      cacheData.AppendElement(
-          BatchData(acc->Document()->IPCDoc(), uid, acc->State(), acc->Bounds(),
-                    name, textValue, nodeID, acc->CurValue(), acc->MinValue(),
-                    acc->MaxValue(), acc->Step(), attributes));
+      cacheData.AppendElement(BatchData(
+          acc->Document()->IPCDoc(), uid, acc->State(), acc->Bounds(),
+          acc->ActionCount(), name, textValue, nodeID, acc->CurValue(),
+          acc->MinValue(), acc->MaxValue(), acc->Step(), attributes));
       mFocusPath.Put(acc->UniqueID(), acc);
     }
 
     ipcDoc->SendBatch(eBatch_FocusPath, cacheData);
   } else if (SessionAccessibility* sessionAcc =
                  SessionAccessibility::GetInstanceFor(this)) {
     nsTArray<AccessibleWrap*> accessibles;
     for (AccessibleWrap* acc = aAccessible; acc && acc != this->Parent();
@@ -228,17 +228,17 @@ void DocAccessibleWrap::UpdateFocusPathB
     DocAccessibleChild* ipcDoc = IPCDoc();
     nsTArray<BatchData> boundsData(mFocusPath.Count());
     for (auto iter = mFocusPath.Iter(); !iter.Done(); iter.Next()) {
       Accessible* accessible = iter.Data();
       auto uid = accessible->IsDoc() && accessible->AsDoc()->IPCDoc()
                      ? 0
                      : reinterpret_cast<uint64_t>(accessible->UniqueID());
       boundsData.AppendElement(BatchData(
-          accessible->Document()->IPCDoc(), uid, 0, accessible->Bounds(),
+          accessible->Document()->IPCDoc(), uid, 0, accessible->Bounds(), 0,
           nsString(), nsString(), nsString(), UnspecifiedNaN<double>(),
           UnspecifiedNaN<double>(), UnspecifiedNaN<double>(),
           UnspecifiedNaN<double>(), nsTArray<Attribute>()));
     }
 
     ipcDoc->SendBatch(eBatch_BoundsUpdate, boundsData);
   } else if (SessionAccessibility* sessionAcc =
                  SessionAccessibility::GetInstanceFor(this)) {
--- a/accessible/android/ProxyAccessibleWrap.cpp
+++ b/accessible/android/ProxyAccessibleWrap.cpp
@@ -75,16 +75,22 @@ void ProxyAccessibleWrap::Value(nsString
 uint64_t ProxyAccessibleWrap::State() { return Proxy()->State(); }
 
 nsIntRect ProxyAccessibleWrap::Bounds() const { return Proxy()->Bounds(); }
 
 void ProxyAccessibleWrap::ScrollTo(uint32_t aHow) const {
   Proxy()->ScrollTo(aHow);
 }
 
+uint8_t
+ProxyAccessibleWrap::ActionCount() const
+{
+  return Proxy()->ActionCount();
+}
+
 // Other
 
 void ProxyAccessibleWrap::SetTextContents(const nsAString& aText) {
   Proxy()->ReplaceText(PromiseFlatString(aText));
 }
 
 void ProxyAccessibleWrap::GetTextContents(nsAString& aText) {
   nsAutoString text;
--- a/accessible/android/ProxyAccessibleWrap.h
+++ b/accessible/android/ProxyAccessibleWrap.h
@@ -40,16 +40,18 @@ class ProxyAccessibleWrap : public Acces
   virtual void Value(nsString& aValue) const override;
 
   virtual uint64_t State() override;
 
   virtual nsIntRect Bounds() const override;
 
   virtual void ScrollTo(uint32_t aHow) const override;
 
+  virtual uint8_t ActionCount() const override;
+
   // AccessibleWrap
 
   virtual void SetTextContents(const nsAString& aText) override;
 
   virtual void GetTextContents(nsAString& aText) override;
 
   virtual bool GetSelectionBounds(int32_t* aStartOffset,
                                   int32_t* aEndOffset) override;
--- a/accessible/android/SessionAccessibility.cpp
+++ b/accessible/android/SessionAccessibility.cpp
@@ -316,17 +316,17 @@ void SessionAccessibility::SendSelectedE
 void SessionAccessibility::ReplaceViewportCache(
     const nsTArray<AccessibleWrap*>& aAccessibles,
     const nsTArray<BatchData>& aData) {
   auto infos = jni::ObjectArray::New<java::GeckoBundle>(aAccessibles.Length());
   for (size_t i = 0; i < aAccessibles.Length(); i++) {
     AccessibleWrap* acc = aAccessibles.ElementAt(i);
     if (aData.Length() == aAccessibles.Length()) {
       const BatchData& data = aData.ElementAt(i);
-      auto bundle = acc->ToSmallBundle(data.State(), data.Bounds());
+      auto bundle = acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount());
       infos->SetElement(i, bundle);
     } else {
       infos->SetElement(i, acc->ToSmallBundle());
     }
   }
 
   mSessionAccessibility->ReplaceViewportCache(infos);
 }
@@ -336,20 +336,20 @@ void SessionAccessibility::ReplaceFocusP
     const nsTArray<BatchData>& aData) {
   auto infos = jni::ObjectArray::New<java::GeckoBundle>(aAccessibles.Length());
   for (size_t i = 0; i < aAccessibles.Length(); i++) {
     AccessibleWrap* acc = aAccessibles.ElementAt(i);
     if (aData.Length() == aAccessibles.Length()) {
       const BatchData& data = aData.ElementAt(i);
       nsCOMPtr<nsIPersistentProperties> props =
           AccessibleWrap::AttributeArrayToProperties(data.Attributes());
-      auto bundle =
-          acc->ToBundle(data.State(), data.Bounds(), data.Name(),
-                        data.TextValue(), data.DOMNodeID(), data.CurValue(),
-                        data.MinValue(), data.MaxValue(), data.Step(), props);
+      auto bundle = acc->ToBundle(
+          data.State(), data.Bounds(), data.ActionCount(), data.Name(),
+          data.TextValue(), data.DOMNodeID(), data.CurValue(), data.MinValue(),
+          data.MaxValue(), data.Step(), props);
       infos->SetElement(i, bundle);
     } else {
       infos->SetElement(i, acc->ToBundle());
     }
   }
 
   mSessionAccessibility->ReplaceFocusPathCache(infos);
 }
@@ -357,17 +357,17 @@ void SessionAccessibility::ReplaceFocusP
 void SessionAccessibility::UpdateCachedBounds(
     const nsTArray<AccessibleWrap*>& aAccessibles,
     const nsTArray<BatchData>& aData) {
   auto infos = jni::ObjectArray::New<java::GeckoBundle>(aAccessibles.Length());
   for (size_t i = 0; i < aAccessibles.Length(); i++) {
     AccessibleWrap* acc = aAccessibles.ElementAt(i);
     if (aData.Length() == aAccessibles.Length()) {
       const BatchData& data = aData.ElementAt(i);
-      auto bundle = acc->ToSmallBundle(data.State(), data.Bounds());
+      auto bundle = acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount());
       infos->SetElement(i, bundle);
     } else {
       infos->SetElement(i, acc->ToSmallBundle());
     }
   }
 
   mSessionAccessibility->UpdateCachedBounds(infos);
 }
--- a/accessible/ipc/other/PDocAccessible.ipdl
+++ b/accessible/ipc/other/PDocAccessible.ipdl
@@ -31,16 +31,17 @@ union OriginDocument
 };
 
 struct BatchData
 {
   OriginDocument Document;
   uint64_t ID;
   uint64_t State;
   nsIntRect Bounds;
+  uint8_t ActionCount;
   nsString Name;
   nsString TextValue;
   nsString DOMNodeID;
   double CurValue;
   double MinValue;
   double MaxValue;
   double Step;
   Attribute[] Attributes;
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
@@ -186,30 +186,32 @@ class AccessibilityTest : BaseSessionTes
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)
             override fun onAccessibilityFocused(event: AccessibilityEvent) {
                 nodeId = getSourceId(event)
                 val node = createNodeInfo(nodeId)
                 assertThat("Label accessibility focused", node.className.toString(),
                         equalTo("android.view.View"))
                 assertThat("Text node should not be focusable", node.isFocusable, equalTo(false))
+                assertThat("Text node should not be clickable", node.isClickable, equalTo(false))
             }
         })
 
         provider.performAction(nodeId,
             AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, null)
 
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)
             override fun onAccessibilityFocused(event: AccessibilityEvent) {
                 nodeId = getSourceId(event)
                 val node = createNodeInfo(nodeId)
                 assertThat("Editbox accessibility focused", node.className.toString(),
                         equalTo("android.widget.EditText"))
                 assertThat("Entry node should be focusable", node.isFocusable, equalTo(true))
+                assertThat("Entry node should be clickable", node.isClickable, equalTo(true))
             }
         })
     }
 
     @Test fun testTextEntryNode() {
         sessionRule.session.loadString("<input aria-label='Name' value='Tobias'>", "text/html")
         waitForInitialFocus()