Add flag to expose ignored nodes to accessibility tree.

This introduces a new base::Feature and corresponding
RuntimeEnabledFeature called AccessibilityExposeIgnoredNodes.

Currently, every AXObject in Blink can be in one of three
states:
1. Normal (not ignored, included in the tree)
2. Ignored and included in the tree
3. Ignored and not included in the tree

Having ignored nodes is important. Having two different
types of ignored nodes is a source of both unnecessary
complexity (causing bugs) and also performance issues -
because we're wasting more time deciding whether to
include the node than it'd take to just include them,
especially when the ignored state changes.

The first step was to expose the HTML element to the
tree, because that required rebaselining nearly every
test. That step is now complete, the
AccessibilityExposeHTMLElement flag is now on by
default and will be removed in the next milestone.

This next step includes all nodes in the tree,
with the exception of ignored static text nodes,
because those can be safely skipped - those typically
represent non-rendered whitespace in the HTML source.
By excluding those from the tree we can avoid
rebaselining 80% of tests.

This patch enables AccessibilityExposeIgnoredNodes
for just one test and rebaselines it. Follow-ups
will enable this flag for all accessibility tests,
rebaselining a few each time, and then flip the
flag.

Bug: 1063155

Change-Id: I74d859a74e75c3f18d71f8e0196f2624d0a98c18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459071
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817822}

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -203,6 +203,22 @@ class DumpAccessibilityTreeTest : public DumpAccessibilityTestBase {
   }
 };
 
+// Subclass of DumpAccessibilityTreeTest that exposes ignored nodes.
+class DumpAccessibilityTreeTestWithIgnoredNodes
+    : public DumpAccessibilityTreeTest {
+ protected:
+  // Override from DumpAccessibilityTreeTest.
+  void ChooseFeatures(std::vector<base::Feature>* enabled_features,
+                      std::vector<base::Feature>* disabled_features) override {
+    // http://crbug.com/1063155 - temporary until this is enabled
+    // everywhere.
+    enabled_features->emplace_back(
+        features::kEnableAccessibilityExposeIgnoredNodes);
+    DumpAccessibilityTreeTest::ChooseFeatures(enabled_features,
+                                              disabled_features);
+  }
+};
+
 void DumpAccessibilityTreeTest::AddDefaultFilters(
     std::vector<AXPropertyFilter>* property_filters) {
   AddPropertyFilter(property_filters, "value='*'");
@@ -243,6 +259,13 @@ INSTANTIATE_TEST_SUITE_P(
                      AccessibilityTreeFormatter::GetTestPasses().size()),
     DumpAccessibilityTreeTestPassToString());
 
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DumpAccessibilityTreeTestWithIgnoredNodes,
+    ::testing::Range(size_t{0},
+                     AccessibilityTreeFormatter::GetTestPasses().size()),
+    DumpAccessibilityTreeTestPassToString());
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityCSSColor) {
   RunCSSTest(FILE_PATH_LITERAL("color.html"));
 }
@@ -1829,7 +1852,7 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityInputRadio) {
   RunHtmlTest(FILE_PATH_LITERAL("input-radio.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTestWithIgnoredNodes,
                        AccessibilityInputRadioCheckboxLabel) {
   RunHtmlTest(FILE_PATH_LITERAL("input-radio-checkbox-label.html"));
 }

content/child/runtime_features.cc

@@ -256,6 +256,8 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
      features::kEnableAccessibilityExposeDisplayNone},
     {wf::EnableAccessibilityExposeHTMLElement,
      features::kEnableAccessibilityExposeHTMLElement},
+    {wf::EnableAccessibilityExposeIgnoredNodes,
+     features::kEnableAccessibilityExposeIgnoredNodes},
     {wf::EnableAllowSyncXHRInPageDismissal,
      blink::features::kAllowSyncXHRInPageDismissal},
     {wf::EnableAutoplayIgnoresWebAudio, media::kAutoplayIgnoreWebAudio},

content/test/data/accessibility/html/input-radio-checkbox-label-expected-android.txt

@@ -5,8 +5,6 @@ android.webkit.WebView focusable focused scrollable
 ++++android.view.View focusable name='label exposed for radio button '
 ++++++android.widget.TextView name='label exposed for radio button '
 ++++++android.widget.RadioButton role_description='radio button' checkable clickable focusable name='label exposed for radio button' item_index=1 row_index=1
-++++++android.widget.TextView name=' '
 ++++android.view.View focusable name='label exposed for checkbox '
 ++++++android.widget.TextView name='label exposed for checkbox '
-++++++android.widget.CheckBox role_description='checkbox' checkable clickable focusable name='label exposed for checkbox'
-++++++android.widget.TextView name=' '
+++++++android.widget.CheckBox role_description='checkbox' checkable clickable focusable name='label exposed for checkbox'

content/test/data/accessibility/html/input-radio-checkbox-label-expected-auralinux.txt

@@ -1,12 +1,10 @@
 [document web]
 ++[section]
-++++[radio button] name='label ignored for radio button' checkable:true
-++++[check box] name='label ignored for checkbox' checkable:true
+++++[radio button] name='label ignored for radio button' labelled-by checkable:true
+++++[check box] name='label ignored for checkbox' labelled-by checkable:true
 ++++[label] name='label exposed for radio button ' label-for
 ++++++[static] name='label exposed for radio button '
 ++++++[radio button] name='label exposed for radio button' labelled-by checkable:true
-++++++[static] name=' '
 ++++[label] name='label exposed for checkbox ' label-for
 ++++++[static] name='label exposed for checkbox '
-++++++[check box] name='label exposed for checkbox' labelled-by checkable:true
-++++++[static] name=' '
+++++++[check box] name='label exposed for checkbox' labelled-by checkable:true

content/test/data/accessibility/html/input-radio-checkbox-label-expected-blink.txt

@@ -1,17 +1,20 @@
 rootWebArea
 ++genericContainer ignored
 ++++genericContainer
-++++++radioButton name='label ignored for radio button' checkedState=false
-++++++checkBox name='label ignored for checkbox' checkedState=false
+++++++labelText ignored
+++++++++staticText ignored name='label ignored for radio button '
+++++++++++inlineTextBox ignored name='label ignored for radio button '
+++++++++radioButton name='label ignored for radio button' checkedState=false
+++++++labelText ignored
+++++++++staticText ignored name='label ignored for checkbox '
+++++++++++inlineTextBox ignored name='label ignored for checkbox '
+++++++++checkBox name='label ignored for checkbox' checkedState=false
 ++++++labelText name='label exposed for radio button '
 ++++++++staticText name='label exposed for radio button '
 ++++++++++inlineTextBox name='label exposed for radio button '
 ++++++++radioButton name='label exposed for radio button' checkedState=false
-++++++++staticText name=' '
-++++++++++inlineTextBox name=' '
 ++++++labelText name='label exposed for checkbox '
 ++++++++staticText name='label exposed for checkbox '
 ++++++++++inlineTextBox name='label exposed for '
 ++++++++++inlineTextBox name='checkbox '
 ++++++++checkBox name='label exposed for checkbox' checkedState=false
-++++++++staticText name=' '

content/test/data/accessibility/html/input-radio-checkbox-label-expected-mac.txt

@@ -5,8 +5,6 @@ AXWebArea
 ++++AXGroup AXTitle='label exposed for radio button '
 ++++++AXStaticText AXValue='label exposed for radio button '
 ++++++AXRadioButton AXTitleUIElement=:5 AXValue=0
-++++++AXStaticText AXValue=' '
 ++++AXGroup AXTitle='label exposed for checkbox '
 ++++++AXStaticText AXValue='label exposed for checkbox '
-++++++AXCheckBox AXTitleUIElement=:9 AXValue=0
-++++++AXStaticText AXValue=' '
+++++++AXCheckBox AXTitleUIElement=:8 AXValue=0

content/test/data/accessibility/html/input-radio-checkbox-label-expected-uia-win.txt

@@ -5,8 +5,6 @@ Document
 ++++Text Name='label exposed for radio button '
 ++++++Text Name='label exposed for radio button '
 ++++++RadioButton Name='label exposed for radio button' SelectionItem.IsSelected=false
-++++++Text Name=' '
 ++++Text Name='label exposed for checkbox '
 ++++++Text Name='label exposed for checkbox '
-++++++CheckBox Name='label exposed for checkbox' Toggle.ToggleState='Off'
-++++++Text Name=' '
+++++++CheckBox Name='label exposed for checkbox' Toggle.ToggleState='Off'

content/test/data/accessibility/html/input-radio-checkbox-label-expected-win.txt

@@ -5,8 +5,6 @@ ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
 ++++IA2_ROLE_LABEL name='label exposed for radio button ' FOCUSABLE
 ++++++ROLE_SYSTEM_STATICTEXT name='label exposed for radio button '
 ++++++ROLE_SYSTEM_RADIOBUTTON name='label exposed for radio button' FOCUSABLE IA2_STATE_CHECKABLE checkable:true
-++++++ROLE_SYSTEM_STATICTEXT name=' '
 ++++IA2_ROLE_LABEL name='label exposed for checkbox ' FOCUSABLE
 ++++++ROLE_SYSTEM_STATICTEXT name='label exposed for checkbox '
-++++++ROLE_SYSTEM_CHECKBUTTON name='label exposed for checkbox' FOCUSABLE IA2_STATE_CHECKABLE checkable:true
-++++++ROLE_SYSTEM_STATICTEXT name=' '
+++++++ROLE_SYSTEM_CHECKBUTTON name='label exposed for checkbox' FOCUSABLE IA2_STATE_CHECKABLE checkable:true

third_party/blink/public/platform/web_runtime_features.h

@@ -74,6 +74,7 @@ class WebRuntimeFeatures {
   BLINK_PLATFORM_EXPORT static void EnableAccelerated2dCanvas(bool);
   BLINK_PLATFORM_EXPORT static void EnableAccessibilityExposeDisplayNone(bool);
   BLINK_PLATFORM_EXPORT static void EnableAccessibilityExposeHTMLElement(bool);
+  BLINK_PLATFORM_EXPORT static void EnableAccessibilityExposeIgnoredNodes(bool);
   BLINK_PLATFORM_EXPORT static void EnableAccessibilityObjectModel(bool);
   BLINK_PLATFORM_EXPORT static void EnableAdTagging(bool);
   BLINK_PLATFORM_EXPORT static void EnableAllowActivationDelegationAttr(bool);

third_party/blink/renderer/modules/accessibility/ax_layout_object.cc

@@ -812,8 +812,8 @@ bool AXLayoutObject::CanIgnoreSpaceNextTo(LayoutObject* layout,
 }
 
 bool AXLayoutObject::CanIgnoreTextAsEmpty() const {
-  DCHECK(layout_object_->IsText());
-  DCHECK(layout_object_->Parent());
+  if (!layout_object_ || !layout_object_->IsText() || !layout_object_->Parent())
+    return false;
 
   LayoutText* layout_text = ToLayoutText(layout_object_);
 

third_party/blink/renderer/modules/accessibility/ax_layout_object.h

@@ -102,6 +102,7 @@ class MODULES_EXPORT AXLayoutObject : public AXNodeObject {
   AXObjectInclusion DefaultObjectInclusion(
       IgnoredReasons* = nullptr) const override;
   bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const override;
+  bool CanIgnoreTextAsEmpty() const override;
 
   // Properties of static elements.
   ax::mojom::blink::ListStyle GetListStyle() const final;
@@ -192,7 +193,6 @@ class MODULES_EXPORT AXLayoutObject : public AXNodeObject {
   bool FindAllTableCellsWithRole(ax::mojom::blink::Role, AXObjectVector&) const;
 
   LayoutRect ComputeElementRect() const;
-  bool CanIgnoreTextAsEmpty() const;
   bool CanIgnoreSpaceNextTo(LayoutObject*, bool is_after) const;
   bool HasAriaCellRole(Element*) const;
   bool IsPlaceholder() const;

third_party/blink/renderer/modules/accessibility/ax_node_object.cc

@@ -505,6 +505,14 @@ bool AXNodeObject::ComputeAccessibilityIsIgnored(
   return true;
 }
 
+bool AXNodeObject::CanIgnoreTextAsEmpty() const {
+  // Note: it's safe to call AXNodeObject::ComputeAccessibilityIsIgnored,
+  // since that has just the logic we need - but note that
+  // AXLayoutObject::ComputeAccessibilityIsIgnored calls CanIgnoreTextAsEmpty
+  // so that'd create a loop.
+  return ComputeAccessibilityIsIgnored();
+}
+
 static bool IsListElement(Node* node) {
   return IsA<HTMLUListElement>(*node) || IsA<HTMLOListElement>(*node) ||
          IsA<HTMLDListElement>(*node);
@@ -3396,10 +3404,23 @@ void AXNodeObject::AddChildren() {
          child = LayoutTreeBuilderTraversal::NextSibling(*child)) {
       if (child->IsMarkerPseudoElement() && AccessibilityIsIgnored())
         continue;
-      AddChild(AXObjectCache().GetOrCreate(child));
+      AXObject* child_obj = AXObjectCache().GetOrCreate(child);
+
+      if (RuntimeEnabledFeatures::AccessibilityExposeIgnoredNodesEnabled() &&
+          child_obj &&
+          child_obj->RoleValue() == ax::mojom::blink::Role::kStaticText &&
+          child_obj->CanIgnoreTextAsEmpty())
+        continue;
+
+      AddChild(child_obj);
     }
   } else {
     for (AXObject* obj = RawFirstChild(); obj; obj = obj->RawNextSibling()) {
+      if (RuntimeEnabledFeatures::AccessibilityExposeIgnoredNodesEnabled() &&
+          obj && obj->RoleValue() == ax::mojom::blink::Role::kStaticText &&
+          obj->CanIgnoreTextAsEmpty())
+        continue;
+
       AddChild(obj);
     }
   }

third_party/blink/renderer/modules/accessibility/ax_node_object.h

@@ -60,6 +60,7 @@ class MODULES_EXPORT AXNodeObject : public AXObject {
   AXObjectInclusion ShouldIncludeBasedOnSemantics(
       IgnoredReasons* = nullptr) const;
   bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const override;
+  bool CanIgnoreTextAsEmpty() const override;
   const AXObject* InheritsPresentationalRoleFrom() const override;
   ax::mojom::blink::Role DetermineTableSectionRole() const;
   ax::mojom::blink::Role DetermineTableCellRole() const;

third_party/blink/renderer/modules/accessibility/ax_object.cc

@@ -1740,6 +1740,9 @@ const AXObject* AXObject::DisabledAncestor() const {
 }
 
 bool AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree() const {
+  if (RuntimeEnabledFeatures::AccessibilityExposeIgnoredNodesEnabled())
+    return true;
+
   if (AXObjectCache().IsAriaOwned(this)) {
     // Always include an aria-owned object. It must be a child of the
     // element with aria-owns.

third_party/blink/renderer/modules/accessibility/ax_object.h

@@ -526,6 +526,7 @@ class MODULES_EXPORT AXObject : public GarbageCollected<AXObject> {
   virtual bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const {
     return true;
   }
+  virtual bool CanIgnoreTextAsEmpty() const { return false; }
   bool AccessibilityIsIgnoredByDefault(IgnoredReasons* = nullptr) const;
   virtual AXObjectInclusion DefaultObjectInclusion(
       IgnoredReasons* = nullptr) const;

third_party/blink/renderer/platform/exported/web_runtime_features.cc

@@ -121,6 +121,10 @@ void WebRuntimeFeatures::EnableAccessibilityExposeHTMLElement(bool enable) {
   RuntimeEnabledFeatures::SetAccessibilityExposeHTMLElementEnabled(enable);
 }
 
+void WebRuntimeFeatures::EnableAccessibilityExposeIgnoredNodes(bool enable) {
+  RuntimeEnabledFeatures::SetAccessibilityExposeIgnoredNodesEnabled(enable);
+}
+
 void WebRuntimeFeatures::EnableAccessibilityObjectModel(bool enable) {
   RuntimeEnabledFeatures::SetAccessibilityObjectModelEnabled(enable);
 }

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -122,6 +122,9 @@
     {
       name: "AccessibilityExposeHTMLElement",
     },
+    {
+      name: "AccessibilityExposeIgnoredNodes",
+    },
     {
       name: "AccessibilityObjectModel",
       status: "experimental",

ui/accessibility/accessibility_features.cc

@@ -28,6 +28,18 @@ bool IsAccessibilityExposeHTMLElementEnabled() {
       ::features::kEnableAccessibilityExposeHTMLElement);
 }
 
+// Enable exposing ignored nodes from Blink to the browser process AXTree.
+// This will allow us to simplify logic by eliminating the distiction between
+// "ignored and included in the tree" from "ignored and not included in the
+// tree".
+const base::Feature kEnableAccessibilityExposeIgnoredNodes{
+    "AccessibilityExposeIgnoredNodes", base::FEATURE_DISABLED_BY_DEFAULT};
+
+bool IsAccessibilityExposeIgnoredNodesEnabled() {
+  return base::FeatureList::IsEnabled(
+      ::features::kEnableAccessibilityExposeIgnoredNodes);
+}
+
 // Enable language detection to determine language used in page text, exposed
 // on the browser process AXTree.
 const base::Feature kEnableAccessibilityLanguageDetection{

ui/accessibility/accessibility_features.h

@@ -24,6 +24,13 @@ AX_BASE_EXPORT extern const base::Feature kEnableAccessibilityExposeHTMLElement;
 // browser process AXTree (as an ignored node).
 AX_BASE_EXPORT bool IsAccessibilityExposeHTMLElementEnabled();
 
+AX_BASE_EXPORT extern const base::Feature
+    kEnableAccessibilityExposeIgnoredNodes;
+
+// Returns true if all ignored nodes are exposed by Blink in the
+// accessibility tree.
+AX_BASE_EXPORT bool IsAccessibilityExposeIgnoredNodesEnabled();
+
 AX_BASE_EXPORT extern const base::Feature kEnableAccessibilityLanguageDetection;
 
 // Return true if language detection should be used to determine the language