Add "aria-describedby" relationship between triggerpopup and hint

For a <button triggerpopup=foo> that points to a <div popup=hint id=foo> this CL will establish a described-by relationship for the button. This is as recommended in this issue/comment: openui/open-ui#329 (comment) AX-Relnotes: When using markup like <button triggerpopup=foo><div popup=hint id=foo>, an accessible description is now computed for the <button> from the content of the <div popup>. This is analogous to when the title attribute results in a description. In addition, in IA2/ATK, the object attribution "description-from" will be set to "popup-element". Bug: 1307772 Change-Id: If562b99af9b829841c038103c0c2dd447cba03d0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648304 Commit-Queue: Mason Freed <[email protected]> Reviewed-by: Nasko Oskov <[email protected]> Reviewed-by: Nektarios Paisios <[email protected]> Reviewed-by: Aaron Leventhal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1005920} NOKEYCHECK=True GitOrigin-RevId: c081b35f7169a5a23446ad455b884ef314dc9b17
mjfroman · May 20, 2022 · d6034e0 · d6034e0
1 parent e7bd282
commit d6034e0
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 39 deletions.
diff --git a/blink/renderer/core/dom/element.cc b/blink/renderer/core/dom/element.cc
@@ -2648,11 +2648,10 @@ const Element* Element::NearestOpenAncestralPopup(Node* start_node) {
       // Case #2 or 3: An anchor or trigger for a showing popup.
       update_highest(anchors_and_invokers.at(current_element));
     } else if (auto* button = DynamicTo<HTMLButtonElement>(current_element)) {
-      if (auto* invoked_popup = button->togglePopupElement()) {
-        if (popup_position.Contains(invoked_popup)) {
-          // Case #4: An invoking element pointing to a showing popup.
-          update_highest(invoked_popup);
-        }
+      if (auto invoked_popup = button->togglePopupElement().element;
+          invoked_popup && popup_position.Contains(invoked_popup)) {
+        // Case #4: An invoking element pointing to a showing popup.
+        update_highest(invoked_popup);
       }
     }
   }

diff --git a/blink/renderer/core/html/forms/html_button_element.cc b/blink/renderer/core/html/forms/html_button_element.cc
@@ -28,6 +28,7 @@
 #include "third_party/blink/renderer/core/dom/attribute.h"
 #include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/events/simulated_click_options.h"
+#include "third_party/blink/renderer/core/dom/qualified_name.h"
 #include "third_party/blink/renderer/core/events/keyboard_event.h"
 #include "third_party/blink/renderer/core/frame/web_feature.h"
 #include "third_party/blink/renderer/core/html/forms/form_data.h"
@@ -110,16 +111,17 @@ void HTMLButtonElement::ParseAttribute(
 
 void HTMLButtonElement::DefaultEventHandler(Event& event) {
   if (event.type() == event_type_names::kDOMActivate) {
-    PopupTriggerAction action;
-    if (Element* popupElement = togglePopupElement(action)) {
-      DCHECK_NE(action, PopupTriggerAction::kNone);
-      if (popupElement->popupOpen() && (action == PopupTriggerAction::kToggle ||
-                                        action == PopupTriggerAction::kHide)) {
-        popupElement->hidePopup(ASSERT_NO_EXCEPTION);
-      } else if (!popupElement->popupOpen() &&
-                 (action == PopupTriggerAction::kToggle ||
-                  action == PopupTriggerAction::kShow)) {
-        popupElement->InvokePopup(this);
+    auto popup = togglePopupElement();
+    if (popup.element) {
+      DCHECK_NE(popup.action, PopupTriggerAction::kNone);
+      if (popup.element->popupOpen() &&
+          (popup.action == PopupTriggerAction::kToggle ||
+           popup.action == PopupTriggerAction::kHide)) {
+        popup.element->hidePopup(ASSERT_NO_EXCEPTION);
+      } else if (!popup.element->popupOpen() &&
+                 (popup.action == PopupTriggerAction::kToggle ||
+                  popup.action == PopupTriggerAction::kShow)) {
+        popup.element->InvokePopup(this);
       }
     }
     if (!IsDisabledFormControl()) {
@@ -148,43 +150,42 @@ void HTMLButtonElement::DefaultEventHandler(Event& event) {
 //  used.
 //  4. If both 'showpopup' and 'hidepopup' are present, the behavior is to
 //  toggle.
-Element* HTMLButtonElement::togglePopupElement(
-    PopupTriggerAction& action) const {
-  action = PopupTriggerAction::kNone;
+HTMLButtonElement::TogglePopupElement HTMLButtonElement::togglePopupElement()
+    const {
+  const TogglePopupElement no_element{nullptr, PopupTriggerAction::kNone,
+                                      g_null_name};
   if (!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled())
-    return nullptr;
+    return no_element;
   if (!IsInTreeScope())
-    return nullptr;
+    return no_element;
   AtomicString idref;
+  QualifiedName attribute_name = html_names::kTogglepopupAttr;
+  PopupTriggerAction action = PopupTriggerAction::kToggle;
   if (FastHasAttribute(html_names::kTogglepopupAttr)) {
     idref = FastGetAttribute(html_names::kTogglepopupAttr);
-    action = PopupTriggerAction::kToggle;
   } else if (FastHasAttribute(html_names::kShowpopupAttr)) {
     idref = FastGetAttribute(html_names::kShowpopupAttr);
     action = PopupTriggerAction::kShow;
+    attribute_name = html_names::kShowpopupAttr;
   }
   if (FastHasAttribute(html_names::kHidepopupAttr)) {
     if (idref.IsNull()) {
       idref = FastGetAttribute(html_names::kHidepopupAttr);
       action = PopupTriggerAction::kHide;
+      attribute_name = html_names::kHidepopupAttr;
     } else if (FastGetAttribute(html_names::kHidepopupAttr) == idref) {
       action = PopupTriggerAction::kToggle;
+      // Leave attribute_name as-is in this case.
     }
   }
   if (idref.IsNull()) {
-    DCHECK_EQ(action, PopupTriggerAction::kNone);
-    return nullptr;
+    return no_element;
   }
   Element* popup_element = GetTreeScope().getElementById(idref);
   if (!popup_element || !popup_element->HasValidPopupAttribute()) {
-    action = PopupTriggerAction::kNone;
-    return nullptr;
+    return no_element;
   }
-  return popup_element;
-}
-Element* HTMLButtonElement::togglePopupElement() const {
-  PopupTriggerAction action;
-  return togglePopupElement(action);
+  return TogglePopupElement{popup_element, action, attribute_name};
 }
 
 bool HTMLButtonElement::HasActivationBehavior() const {

diff --git a/blink/renderer/core/html/forms/html_button_element.h b/blink/renderer/core/html/forms/html_button_element.h
@@ -45,10 +45,18 @@ class CORE_EXPORT HTMLButtonElement final : public HTMLFormControlElement {
                          mojom::blink::FocusType,
                          InputDeviceCapabilities*) override;
 
+  struct TogglePopupElement final {
+   public:
+    DISALLOW_NEW();
+    WeakMember<Element> element;
+    PopupTriggerAction action;
+    QualifiedName attribute_name;
+    void Trace(Visitor* visitor) const { visitor->Trace(element); }
+  };
+
   // Retrieves the element pointed to by 'togglepopup', 'showpopup', and/or
   // 'hidepopup' content attributes, if any.
-  Element* togglePopupElement(PopupTriggerAction& action) const;
-  Element* togglePopupElement() const;
+  TogglePopupElement togglePopupElement() const;
 
  private:
   enum Type { kSubmit, kReset, kButton };

diff --git a/blink/renderer/modules/accessibility/ax_node_object.cc b/blink/renderer/modules/accessibility/ax_node_object.cc
@@ -1841,10 +1841,9 @@ AccessibilityExpanded AXNodeObject::IsExpanded() const {
   // with type kPopup, then set aria-expanded=false when the popup is hidden,
   // and aria-expanded=true when it is showing.
   if (auto* button = DynamicTo<HTMLButtonElement>(element)) {
-    if (auto* popup = button->togglePopupElement()) {
-      if (popup->PopupType() == PopupValueType::kPopup) {
-        return popup->popupOpen() ? kExpandedExpanded : kExpandedCollapsed;
-      }
+    if (auto popup = button->togglePopupElement().element;
+        popup && popup->PopupType() == PopupValueType::kPopup) {
+      return popup->popupOpen() ? kExpandedExpanded : kExpandedCollapsed;
     }
   }
 
@@ -3236,8 +3235,7 @@ String AXNodeObject::TextAlternative(
     NameSources* name_sources) const {
   // If nameSources is non-null, relatedObjects is used in filling it in, so it
   // must be non-null as well.
-  if (name_sources)
-    DCHECK(related_objects);
+  DCHECK(!name_sources || related_objects);
 
   bool found_text_alternative = false;
 
@@ -5596,6 +5594,40 @@ String AXNodeObject::Description(
     }
   }
 
+  // For buttons that contain the |togglepopup|, |showpopup|, or |hidepopup|
+  // popup triggering attributes, and the pointed-to element is a valid popup
+  // with type kHint, then set aria-describedby to the hint popup.
+  if (auto* button = DynamicTo<HTMLButtonElement>(element)) {
+    auto popup = button->togglePopupElement();
+    if (popup.element && popup.element->PopupType() == PopupValueType::kHint) {
+      description_from = ax::mojom::blink::DescriptionFrom::kPopupElement;
+      if (description_sources) {
+        description_sources->push_back(
+            DescriptionSource(found_description, popup.attribute_name));
+        description_sources->back().type = description_from;
+      }
+      AXObject* popup_ax_object = AXObjectCache().GetOrCreate(popup.element);
+      if (popup_ax_object) {
+        AXObjectSet visited;
+        description = RecursiveTextAlternative(*popup_ax_object,
+                                               popup_ax_object, visited);
+        if (related_objects) {
+          related_objects->push_back(
+              MakeGarbageCollected<NameSourceRelatedObject>(popup_ax_object,
+                                                            description));
+        }
+        if (description_sources) {
+          DescriptionSource& source = description_sources->back();
+          source.related_objects = *related_objects;
+          source.text = description;
+          found_description = true;
+        } else {
+          return description;
+        }
+      }
+    }
+  }
+
   description_from = ax::mojom::blink::DescriptionFrom::kNone;
 
   if (found_description) {

diff --git a/closure_compiler/externs/automation.js b/closure_compiler/externs/automation.js
@@ -1,4 +1,4 @@
-// Copyright 2021 The Chromium Authors. All rights reserved.
+// Copyright 2022 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.
 
@@ -462,6 +462,7 @@ chrome.automation.NameFromType = {
 chrome.automation.DescriptionFromType = {
   ARIA_DESCRIPTION: 'ariaDescription',
   BUTTON_LABEL: 'buttonLabel',
+  POPUP_ELEMENT: 'popupElement',
   RELATED_ELEMENT: 'relatedElement',
   RUBY_ANNOTATION: 'rubyAnnotation',
   SUMMARY: 'summary',