Bug 272840 – AX: AccessibilityMenuListOption should subclass AccessibilityNodeObject

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Tyler Wilcock]

It is a node object, and the fact that it doesn't subclass AccessibilityNodeObject means it's missing important overrides like accessibilityText.

[Radar WebKit Bug Importer]

<rdar://problem/126634909>

[Tyler Wilcock]

Created attachment 470964 [details]
Patch

[Tyler Wilcock]

Created attachment 470971 [details]
Patch

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #3)
> Created attachment 470971 [details]
> Patch

diff --git a/Source/WebCore/accessibility/AccessibilityMenuListOption.cpp b/Source/WebCore/accessibility/AccessibilityMenuListOption.cpp
index e76d336b9639..8c752ba71482 100644
--- a/Source/WebCore/accessibility/AccessibilityMenuListOption.cpp
+++ b/Source/WebCore/accessibility/AccessibilityMenuListOption.cpp
@@ -38,7 +38,7 @@ namespace WebCore {
 using namespace HTMLNames;

 AccessibilityMenuListOption::AccessibilityMenuListOption(HTMLOptionElement& element)
-    : m_element(element)
+    : AccessibilityNodeObject(&element)
     , m_parent(nullptr)
 {
 }
@@ -48,28 +48,30 @@ Ref<AccessibilityMenuListOption> AccessibilityMenuListOption::create(HTMLOptionE
     return adoptRef(*new AccessibilityMenuListOption(element));
 }

-Element* AccessibilityMenuListOption::actionElement() const
+HTMLOptionElement* AccessibilityMenuListOption::optionElement() const
 {
-    return m_element.get();
+    return downcast<HTMLOptionElement>(node());
 }

-Node* AccessibilityMenuListOption::node() const
+Element* AccessibilityMenuListOption::actionElement() const
 {
-    return m_element.get();
+    return downcast<Element>(node());
 }
-
+
 bool AccessibilityMenuListOption::isEnabled() const
 {
-    return m_element && !m_element->ownElementDisabled();
+    auto* optionElement = this->optionElement();
+    return optionElement && !optionElement->ownElementDisabled();
 }

 bool AccessibilityMenuListOption::isVisible() const
 {
-    if (!m_element)
+    WeakPtr optionElement = this->optionElement();
+    if (!optionElement)
         return false;

     // In a single-option select with the popup collapsed, only the selected item is considered visible.
-    auto ownerSelectElement = m_element->document().axObjectCache()->getOrCreate(m_element->ownerSelectElement());
+    auto ownerSelectElement = optionElement->document().axObjectCache()->getOrCreate(optionElement->ownerSelectElement());

AG: auto* ?

     return ownerSelectElement && (!ownerSelectElement->isOffScreen() || isSelected());
 }

@@ -81,7 +83,8 @@ bool AccessibilityMenuListOption::isOffScreen() const

 bool AccessibilityMenuListOption::isSelected() const
 {
-    return m_element && m_element->selected();
+    auto* optionElement = this->optionElement();
+    return optionElement && optionElement->selected();
 }

 void AccessibilityMenuListOption::setSelected(bool selected)
@@ -89,7 +92,8 @@ void AccessibilityMenuListOption::setSelected(bool selected)
     if (!canSetSelectedAttribute())
         return;

-    m_element->setSelected(selected);
+    if (auto* optionElement = this->optionElement())
+        optionElement->setSelected(selected);
 }

 bool AccessibilityMenuListOption::canSetSelectedAttribute() const
@@ -104,23 +108,22 @@ bool AccessibilityMenuListOption::computeAccessibilityIsIgnored() const

 LayoutRect AccessibilityMenuListOption::elementRect() const
 {
-    AccessibilityObject* parent = parentObject();
+    RefPtr parent = parentObject();
     // Our parent should've been set to be a menu-list popup before this method is called.
     ASSERT(parent && parent->isMenuListPopup());
     if (!parent)
         return boundingBoxRect();

-    AccessibilityObject* grandparent = parent->parentObject();
-    if (!grandparent)
-        return boundingBoxRect();
-    ASSERT(grandparent->isMenuList());
+    RefPtr grandparent = parent->parentObject();
+    ASSERT(!grandparent || grandparent->isMenuList());

-    return grandparent->elementRect();
+    return grandparent ? grandparent->elementRect() : boundingBoxRect();
 }

 String AccessibilityMenuListOption::stringValue() const
 {
-    return m_element ? m_element->label() : String();
+    auto* optionElement = this->optionElement();
+    return optionElement ? optionElement->label() : String();
 }

 } // namespace WebCore

[Tyler Wilcock]

Created attachment 470976 [details]
Patch

[EWS]

Committed 277680@main (c87f99943fbe): <https://commits.webkit.org/277680@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 470976 [details].