Add logic to expose "display: none" nodes to accessibility behind flag

This is CL [3/4] in a series of CLs to expose "display: none"
elements to the browser process AXTree.

1. Add and improve tests for CSS display and visibility.
2. Introduce new command line flag and chrome://flags entry.
3. Update AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree
   and relevant logic for the feature to work, disabled using the
   feature flag.
4. Enable the feature in tests; update expectation files.

---

This change hooks up everything required to expose nodes that are
"display: none" to be exposed to the browser-side AX tree with the
State::kIgnored, but behind a RuntimeEnabledFeatures.
Doing so allows elements with "display: none" to be returned from
UIA_DescribedByPropertyId and UIA_LabeledByPropertyId.
This change can be observed in the update to
AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree().

---

There was a bug with AXNodePosition::AsUnignoredTextPosition when a
leaf text position was the first or last in a document.
It would fail to find the next/previous unignored position because it
only searched in one direction. In the case where the last node in a
document is ignored, we must search in reverse order if no unignored
position can be found in the forward direction.

---

In response to AXEventGenerator::Event::IGNORED_CHANGED, we now also
need to fire menu opened / closed events for MSAA and UIA. This is
because when a menu goes from ignored to unignored it is exposed to
the accessibility tree as a new node, similar to if a new unignored
menu was added to the DOM via script.

---

Modifying AXObjectCacheImpl::HandleRoleChangeWithCleanLayout so that
it will create the AXObject if it doesn't exist already. This fixes an
invalidation issue when changing the role of an element where the
parent object may not be invalidated for a change in its children.

This is needed to fix :
blink/web_tests/accessibility/is-ignored-change-sends-notification.html

I tested |divWithoutRoleContainer| in isolation without my change and
it appears that test case was providing a false-positive before.

---

Deferring ax::mojom::Event::kBlur and ::kFocus when calling
|AXObjectCacheImpl::HandleFocusedUIElementChanged|.
I needed to defer this event due to my changes in
|AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree|.

This is because the following stack will have a DCHECK failure :
Check failed: !GetDocument().NeedsLayoutTreeUpdateForNode( *this, true )
```
blink::Element::EnsureComputedStyle
blink::AXObject::IsHiddenForTextAlternativeCalculation
blink::AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree
blink::AXObject::UpdateCachedAttributeValuesIfNeeded
blink::AXObject::AccessibilityIsIgnored
blink::AXObjectCacheImpl::GetOrCreate
blink::AXObjectCacheImpl::FocusedObject
blink::AXObjectCacheImpl::HandleFocusedUIElementChanged
```

---

Bug: 651614
Change-Id: Ieef8e43d19de147beb827139e761085b1d13d0e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808430
Commit-Queue: Adam Ettenberger <adettenb@microsoft.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#699012}

content/browser/accessibility/accessibility_event_recorder_auralinux.cc

@@ -222,12 +222,15 @@ void AccessibilityEventRecorderAuraLinux::ProcessATKEvent(
           base::NumberToString(g_value_get_double(&property_values->new_value));
     } else if (g_strcmp0(property_values->property_name, "accessible-name") ==
                0) {
+      const char* new_name = g_value_get_string(&property_values->new_value);
       log += "NAME-CHANGED:";
-      log += g_value_get_string(&property_values->new_value);
+      log += (new_name) ? new_name : "(null)";
     } else if (g_strcmp0(property_values->property_name,
                          "accessible-description") == 0) {
+      const char* new_description =
+          g_value_get_string(&property_values->new_value);
       log += "DESCRIPTION-CHANGED:";
-      log += g_value_get_string(&property_values->new_value);
+      log += (new_description) ? new_description : "(null)";
     } else {
       return;
     }

content/browser/accessibility/browser_accessibility_manager_win.cc

@@ -242,9 +242,17 @@ void BrowserAccessibilityManagerWin::FireGeneratedEvent(
       if (node->IsIgnored()) {
         FireWinAccessibilityEvent(EVENT_OBJECT_HIDE, node);
         FireUiaStructureChangedEvent(StructureChangeType_ChildRemoved, node);
+        if (node->GetRole() == ax::mojom::Role::kMenu) {
+          FireWinAccessibilityEvent(EVENT_SYSTEM_MENUPOPUPEND, node);
+          FireUiaAccessibilityEvent(UIA_MenuClosedEventId, node);
+        }
       } else {
         FireWinAccessibilityEvent(EVENT_OBJECT_SHOW, node);
         FireUiaStructureChangedEvent(StructureChangeType_ChildAdded, node);
+        if (node->GetRole() == ax::mojom::Role::kMenu) {
+          FireWinAccessibilityEvent(EVENT_SYSTEM_MENUPOPUPSTART, node);
+          FireUiaAccessibilityEvent(UIA_MenuOpenedEventId, node);
+        }
       }
       aria_properties_events_.insert(node);
       break;
@@ -411,11 +419,15 @@ void BrowserAccessibilityManagerWin::FireWinAccessibilityEvent(
     return;
   if (!ShouldFireEventForNode(node))
     return;
-  // Suppress events when |IGNORED_CHANGED| except for related SHOW / HIDE
+  // Suppress events when |IGNORED_CHANGED| except for related SHOW / HIDE.
+  // Also include MENUPOPUPSTART / MENUPOPUPEND since a change in the ignored
+  // state may show / hide a popup by exposing it to the tree or not.
   if (base::Contains(ignored_changed_nodes_, node)) {
     switch (win_event_type) {
       case EVENT_OBJECT_HIDE:
       case EVENT_OBJECT_SHOW:
+      case EVENT_SYSTEM_MENUPOPUPEND:
+      case EVENT_SYSTEM_MENUPOPUPSTART:
         break;
       default:
         return;
@@ -443,9 +455,20 @@ void BrowserAccessibilityManagerWin::FireUiaAccessibilityEvent(
     return;
   if (!ShouldFireEventForNode(node))
     return;
-  // Suppress events when |IGNORED_CHANGED|
-  if (node->IsIgnored() || base::Contains(ignored_changed_nodes_, node))
+  // Suppress events when |IGNORED_CHANGED| except for MenuClosed / MenuOpen
+  // since a change in the ignored state may show / hide a popup by exposing
+  // it to the tree or not.
+  if (base::Contains(ignored_changed_nodes_, node)) {
+    switch (uia_event) {
+      case UIA_MenuClosedEventId:
+      case UIA_MenuOpenedEventId:
+        break;
+      default:
+        return;
+    }
+  } else if (node->IsIgnored()) {
     return;
+  }
 
   ::UiaRaiseAutomationEvent(ToBrowserAccessibilityWin(node)->GetCOM(),
                             uia_event);

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

@@ -56,6 +56,7 @@
 #include "third_party/blink/renderer/modules/accessibility/ax_range.h"
 #include "third_party/blink/renderer/modules/accessibility/ax_sparse_attribute_setter.h"
 #include "third_party/blink/renderer/platform/language.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/text/platform_locale.h"
 #include "third_party/blink/renderer/platform/wtf/hash_set.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
@@ -1213,16 +1214,29 @@ bool AXObject::ComputeAccessibilityIsIgnoredButIncludedInTree() const {
   if (IsLineBreakingObject())
     return true;
 
-  // Allow the browser side ax tree to access aria-hidden="true", "visibility:
-  // hidden", and "visibility: collapse" nodes. This is useful for APIs that
-  // return the node referenced by aria-labeledby and aria-describedby
-  if (GetLayoutObject()) {
+  // Allow the browser side ax tree to access "visibility: [hidden|collapse]"
+  // and "display: none" nodes. This is useful for APIs that return the node
+  // referenced by aria-labeledby and aria-describedby.
+  // An element must have an id attribute or it cannot be referenced by
+  // aria-labelledby or aria-describedby.
+  if (RuntimeEnabledFeatures::AccessibilityExposeDisplayNoneEnabled()) {
+    if (Element* element = GetElement()) {
+      if (element->FastHasAttribute(kIdAttr) &&
+          IsHiddenForTextAlternativeCalculation()) {
+        return true;
+      }
+    }
+  } else if (GetLayoutObject()) {
     if (GetLayoutObject()->Style()->Visibility() != EVisibility::kVisible)
       return true;
-    if (AriaHiddenRoot())
-      return true;
   }
 
+  // Allow the browser side ax tree to access "aria-hidden" nodes.
+  // This is useful for APIs that return the node referenced by
+  // aria-labeledby and aria-describedby.
+  if (GetLayoutObject() && AriaHiddenRoot())
+    return true;
+
   return false;
 }
 

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

@@ -88,6 +88,7 @@
 #include "third_party/blink/renderer/modules/accessibility/ax_virtual_object.h"
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_elements_helper.h"
 #include "third_party/blink/renderer/modules/permissions/permission_utils.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 
 namespace blink {
 
@@ -168,6 +169,45 @@ void AXObjectCacheImpl::DisposePopup(Document* document) {
   documents_.erase(document);
 }
 
+Node* AXObjectCacheImpl::FocusedElement() {
+  Node* focused_node = document_->FocusedElement();
+  if (!focused_node)
+    focused_node = document_;
+
+  // If it's an image map, get the focused link within the image map.
+  if (IsA<HTMLAreaElement>(focused_node))
+    return focused_node;
+
+  // See if there's a page popup, for example a calendar picker.
+  Element* adjusted_focused_element = document_->AdjustedFocusedElement();
+  if (auto* input = ToHTMLInputElementOrNull(adjusted_focused_element)) {
+    if (AXObject* ax_popup = input->PopupRootAXObject()) {
+      if (Element* focused_element_in_popup =
+              ax_popup->GetDocument()->FocusedElement())
+        focused_node = focused_element_in_popup;
+    }
+  }
+
+  return focused_node;
+}
+
+AXObject* AXObjectCacheImpl::GetOrCreateFocusedObjectFromNode(Node* node) {
+  // If it's an image map, get the focused link within the image map.
+  if (auto* area = DynamicTo<HTMLAreaElement>(node))
+    return FocusedImageMapUIElement(area);
+
+  AXObject* obj = GetOrCreate(node);
+  if (!obj)
+    return nullptr;
+
+  // the HTML element, for example, is focusable but has an AX object that is
+  // ignored
+  if (!obj->AccessibilityIsIncludedInTree())
+    obj = obj->ParentObjectIncludedInTree();
+
+  return obj;
+}
+
 AXObject* AXObjectCacheImpl::FocusedImageMapUIElement(
     HTMLAreaElement* area_element) {
   // Find the corresponding accessibility object for the HTMLAreaElement. This
@@ -198,34 +238,7 @@ AXObject* AXObjectCacheImpl::FocusedImageMapUIElement(
 }
 
 AXObject* AXObjectCacheImpl::FocusedObject() {
-  Node* focused_node = document_->FocusedElement();
-  if (!focused_node)
-    focused_node = document_;
-
-  // If it's an image map, get the focused link within the image map.
-  if (auto* area = DynamicTo<HTMLAreaElement>(focused_node))
-    return FocusedImageMapUIElement(area);
-
-  // See if there's a page popup, for example a calendar picker.
-  Element* adjusted_focused_element = document_->AdjustedFocusedElement();
-  if (auto* input = ToHTMLInputElementOrNull(adjusted_focused_element)) {
-    if (AXObject* ax_popup = input->PopupRootAXObject()) {
-      if (Element* focused_element_in_popup =
-              ax_popup->GetDocument()->FocusedElement())
-        focused_node = focused_element_in_popup;
-    }
-  }
-
-  AXObject* obj = GetOrCreate(focused_node);
-  if (!obj)
-    return nullptr;
-
-  // the HTML element, for example, is focusable but has an AX object that is
-  // ignored
-  if (!obj->AccessibilityIsIncludedInTree())
-    obj = obj->ParentObjectUnignored();
-
-  return obj;
+  return GetOrCreateFocusedObjectFromNode(this->FocusedElement());
 }
 
 AXObject* AXObjectCacheImpl::Get(LayoutObject* layout_object) {
@@ -1175,6 +1188,30 @@ void AXObjectCacheImpl::HandleAriaSelectedChangedWithCleanLayout(Node* node) {
     PostNotification(listbox, ax::mojom::Event::kSelectedChildrenChanged);
 }
 
+void AXObjectCacheImpl::HandleNodeLostFocusWithCleanLayout(Node* node) {
+  DCHECK(node);
+  DCHECK(!node->GetDocument().NeedsLayoutTreeUpdateForNode(*node));
+  AXObject* obj = Get(node);
+  if (!obj)
+    return;
+
+  PostNotification(obj, ax::mojom::Event::kBlur);
+}
+
+void AXObjectCacheImpl::HandleNodeGainedFocusWithCleanLayout(Node* node) {
+  DCHECK(node);
+  DCHECK(!node->GetDocument().NeedsLayoutTreeUpdateForNode(*node));
+  // Something about the call chain for this method seems to leave distribution
+  // in a dirty state - update it before we call GetOrCreate so that we don't
+  // crash.
+  node->UpdateDistributionForFlatTreeTraversal();
+  AXObject* obj = GetOrCreateFocusedObjectFromNode(node);
+  if (!obj)
+    return;
+
+  PostNotification(obj, ax::mojom::Event::kFocus);
+}
+
 // This might be the new target of a relation. Handle all possible cases.
 void AXObjectCacheImpl::MaybeNewRelationTarget(Node* node, AXObject* obj) {
   // Track reverse relations
@@ -1219,12 +1256,8 @@ void AXObjectCacheImpl::HandleRoleChangeWithCleanLayout(Node* node) {
 
   DCHECK(!node->GetDocument().NeedsLayoutTreeUpdateForNode(*node));
 
-  AXObject* obj = Get(node);
-  if (!obj && IsHTMLSelectElement(node))
-    obj = GetOrCreate(node);
-
   // Invalidate the current object and make the parent reconsider its children.
-  if (obj) {
+  if (AXObject* obj = GetOrCreate(node)) {
     // Save parent for later use.
     AXObject* parent = obj->ParentObject();
 
@@ -1590,13 +1623,23 @@ void AXObjectCacheImpl::HandleFocusedUIElementChanged(
   if (!page)
     return;
 
-  AXObject* focused_object = this->FocusedObject();
-  if (!focused_object)
-    return;
+  if (RuntimeEnabledFeatures::AccessibilityExposeDisplayNoneEnabled()) {
+    if (old_focused_element) {
+      DeferTreeUpdate(&AXObjectCacheImpl::HandleNodeLostFocusWithCleanLayout,
+                      old_focused_element);
+    }
 
-  AXObject* old_focused_object = Get(old_focused_element);
-  PostNotification(old_focused_object, ax::mojom::Event::kBlur);
-  PostNotification(focused_object, ax::mojom::Event::kFocus);
+    DeferTreeUpdate(&AXObjectCacheImpl::HandleNodeGainedFocusWithCleanLayout,
+                    this->FocusedElement());
+  } else {
+    AXObject* focused_object = this->FocusedObject();
+    if (!focused_object)
+      return;
+
+    AXObject* old_focused_object = Get(old_focused_element);
+    PostNotification(old_focused_object, ax::mojom::Event::kBlur);
+    PostNotification(focused_object, ax::mojom::Event::kFocus);
+  }
 }
 
 void AXObjectCacheImpl::HandleInitialFocus() {

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

@@ -194,6 +194,8 @@ class MODULES_EXPORT AXObjectCacheImpl
   void HandleRoleChangeIfNotEditableWithCleanLayout(Node*);
   void HandleAriaExpandedChangeWithCleanLayout(Node*);
   void HandleAriaSelectedChangedWithCleanLayout(Node*);
+  void HandleNodeLostFocusWithCleanLayout(Node*);
+  void HandleNodeGainedFocusWithCleanLayout(Node*);
 
   bool InlineTextBoxAccessibilityEnabled();
 
@@ -322,6 +324,12 @@ class MODULES_EXPORT AXObjectCacheImpl
   // ContextLifecycleObserver overrides.
   void ContextDestroyed(ExecutionContext*) override;
 
+  // Get the currently focused Node element.
+  Node* FocusedElement();
+
+  // GetOrCreate the focusable AXObject for a specific Node.
+  AXObject* GetOrCreateFocusedObjectFromNode(Node*);
+
   AXObject* FocusedImageMapUIElement(HTMLAreaElement*);
 
   AXID GetOrCreateAXID(AXObject*);

ui/accessibility/ax_node_position.cc

@@ -5,6 +5,7 @@
 #include "ui/accessibility/ax_node_position.h"
 
 #include "base/strings/string_util.h"
+#include "ui/accessibility/accessibility_features.h"
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_tree_manager_map.h"
 
@@ -92,32 +93,24 @@ AXNodePosition::AXPositionInstance AXNodePosition::AsUnignoredTextPosition(
   if (!IsLeafTextPosition())
     return AsLeafTextPosition()->AsUnignoredTextPosition(adjustment_behavior);
 
-  if (!GetAnchor()->IsIgnored())
-    return Clone();
-
-  // Find the next/previous node that is not ignored.
-  AXNode* unignored_node = GetAnchor();
-  while (unignored_node) {
-    switch (adjustment_behavior) {
-      case AdjustmentBehavior::kMoveRight:
-        unignored_node = unignored_node->GetNextUnignoredInTreeOrder();
-        break;
-      case AdjustmentBehavior::kMoveLeft:
-        unignored_node = unignored_node->GetPreviousUnignoredInTreeOrder();
-    }
-    if (unignored_node && unignored_node->IsText()) {
-      switch (adjustment_behavior) {
-        case AdjustmentBehavior::kMoveRight:
-          return CreateTextPosition(tree_id(), unignored_node->id(), 0,
-                                    ax::mojom::TextAffinity::kDownstream);
-        case AdjustmentBehavior::kMoveLeft:
-          return CreateTextPosition(tree_id(), unignored_node->id(), 0,
-                                    ax::mojom::TextAffinity::kDownstream)
-              ->CreatePositionAtEndOfAnchor();
-      }
+  AXPositionInstance unignored_position =
+      CreateUnignoredPositionFromLeafTextPosition(adjustment_behavior);
+
+  // If creating an unignored position using |adjustment_behavior| returns a
+  // null position, the position may be at the start or end of a document.
+  // For this case attempt to adjust using the opposite AdjustmentBehavior.
+  if (features::IsAccessibilityExposeDisplayNoneEnabled()) {
+    if (unignored_position->IsNullPosition()) {
+      const AdjustmentBehavior opposite_adjustment =
+          (adjustment_behavior == AdjustmentBehavior::kMoveRight)
+              ? AdjustmentBehavior::kMoveLeft
+              : AdjustmentBehavior::kMoveRight;
+      unignored_position =
+          CreateUnignoredPositionFromLeafTextPosition(opposite_adjustment);
     }
   }
-  return CreateNullPosition();
+
+  return unignored_position;
 }
 
 int AXNodePosition::MaxTextOffset() const {
@@ -372,4 +365,37 @@ AXNode* AXNodePosition::GetParent(AXNode* child,
   return parent;
 }
 
+AXNodePosition::AXPositionInstance
+AXNodePosition::CreateUnignoredPositionFromLeafTextPosition(
+    AdjustmentBehavior adjustment_behavior) const {
+  DCHECK(IsLeafTextPosition());
+
+  AXNode* unignored_node = GetAnchor();
+  if (!unignored_node->IsIgnored())
+    return Clone();
+
+  // Find the next/previous node that is not ignored.
+  while (unignored_node) {
+    switch (adjustment_behavior) {
+      case AdjustmentBehavior::kMoveRight:
+        unignored_node = unignored_node->GetNextUnignoredInTreeOrder();
+        break;
+      case AdjustmentBehavior::kMoveLeft:
+        unignored_node = unignored_node->GetPreviousUnignoredInTreeOrder();
+    }
+    if (unignored_node && unignored_node->IsText()) {
+      switch (adjustment_behavior) {
+        case AdjustmentBehavior::kMoveRight:
+          return CreateTextPosition(tree_id(), unignored_node->id(), 0,
+                                    ax::mojom::TextAffinity::kDownstream);
+        case AdjustmentBehavior::kMoveLeft:
+          return CreateTextPosition(tree_id(), unignored_node->id(), 0,
+                                    ax::mojom::TextAffinity::kDownstream)
+              ->CreatePositionAtEndOfAnchor();
+      }
+    }
+  }
+  return CreateNullPosition();
+}
+
 }  // namespace ui

ui/accessibility/ax_node_position.h

@@ -73,6 +73,9 @@ class AX_EXPORT AXNodePosition : public AXPosition<AXNodePosition, AXNode> {
                            AXTreeID child_tree_id,
                            AXTreeID* parent_tree_id,
                            int32_t* parent_id);
+
+  AXPositionInstance CreateUnignoredPositionFromLeafTextPosition(
+      AdjustmentBehavior adjustment_behavior) const;
 };
 
 }  // namespace ui