AX: Make AccessibilityListBoxOption derive from AccessibilityNodeObject.

https://bugs.webkit.org/show_bug.cgi?id=269427
<rdar://problem/122987407>

Reviewed by Tyler Wilcock.

This allows to use downcast in the AccessibilityNodeObject implementation when the object is actually a ListBoxOption. This provieds several advantages in the implementation of methods such as AccessibilityNodeObject::stringValue() and text().

This fixes an issue of VoiceOver not speaking some option elements in <select>.

In addition, there is no downside to doing this since previously the ListBoxOption was keeping a reference to an HTMLElement that was either downcast to an HTMLOptionElement or HTmLOptGroupElement. Now we do the same downcasting of a Node object. Some code cleanup included.

* LayoutTests/inspector/dom/getAccessibilityPropertiesForNode-expected.txt:
* LayoutTests/platform/gtk/inspector/dom/getAccessibilityPropertiesForNode-expected.txt:
* Source/WebCore/accessibility/AccessibilityListBoxOption.cpp:
(WebCore::AccessibilityListBoxOption::AccessibilityListBoxOption):
(WebCore::AccessibilityListBoxOption::isEnabled const):
(WebCore::AccessibilityListBoxOption::isSelected const):
(WebCore::AccessibilityListBoxOption::elementRect const):
(WebCore::AccessibilityListBoxOption::computeAccessibilityIsIgnored const):
(WebCore::AccessibilityListBoxOption::canSetSelectedAttribute const):
(WebCore::AccessibilityListBoxOption::stringValue const):
(WebCore::AccessibilityListBoxOption::actionElement const):
(WebCore::AccessibilityListBoxOption::node const):
(WebCore::AccessibilityListBoxOption::parentObject const):
(WebCore::AccessibilityListBoxOption::listBoxOptionParentNode const):
(WebCore::AccessibilityListBoxOption::listBoxOptionIndex const):
* Source/WebCore/accessibility/AccessibilityListBoxOption.h:
* Source/WebCore/accessibility/AccessibilityNodeObject.h:

Canonical link: https://commits.webkit.org/274795@main

LayoutTests/inspector/dom/getAccessibilityPropertiesForNode-expected.txt

@@ -312,13 +312,13 @@ Total elements to be tested: 123.
 
 <option>not selected</option>
     exists: true
-    label:
+    label: not selected
     role: option
     parentNodeId: exists
 
 <option selected="">selected</option>
     exists: true
-    label:
+    label: selected
     role: option
     parentNodeId: exists
     selected: true

LayoutTests/platform/gtk/inspector/dom/getAccessibilityPropertiesForNode-expected.txt

@@ -297,13 +297,13 @@ Total elements to be tested: 123.
 
 <option>not selected</option>
     exists: true
-    label:
+    label: not selected
     role: option
     parentNodeId: exists
 
 <option selected="">selected</option>
     exists: true
-    label:
+    label: selected
     role: option
     parentNodeId: exists
     selected: true

Source/WebCore/accessibility/AccessibilityListBoxOption.cpp

@@ -42,39 +42,29 @@
 namespace WebCore {
 
 using namespace HTMLNames;
-    
+
 AccessibilityListBoxOption::AccessibilityListBoxOption(HTMLElement& element)
-    : m_optionElement(element)
+    : AccessibilityNodeObject(&element)
 {
 }
 
 AccessibilityListBoxOption::~AccessibilityListBoxOption() = default;
-    
+
 Ref<AccessibilityListBoxOption> AccessibilityListBoxOption::create(HTMLElement& element)
 {
     return adoptRef(*new AccessibilityListBoxOption(element));
 }
-    
+
 bool AccessibilityListBoxOption::isEnabled() const
 {
-    if (is<HTMLOptGroupElement>(m_optionElement))
-        return false;
-
-    if (equalLettersIgnoringASCIICase(getAttribute(aria_disabledAttr), "true"_s))
-        return false;
-
-    if (m_optionElement->hasAttributeWithoutSynchronization(disabledAttr))
-        return false;
-
-    return true;
+    return !(is<HTMLOptGroupElement>(m_node.get())
+        || equalLettersIgnoringASCIICase(getAttribute(aria_disabledAttr), "true"_s)
+        || hasAttribute(disabledAttr));
 }
-    
+
 bool AccessibilityListBoxOption::isSelected() const
 {
-    if (!m_optionElement)
-        return false;
-
-    RefPtr option = dynamicDowncast<HTMLOptionElement>(*m_optionElement);
+    RefPtr option = dynamicDowncast<HTMLOptionElement>(m_node.get());
     return option && option->selected();
 }
 
@@ -89,29 +79,32 @@ bool AccessibilityListBoxOption::isSelectedOptionActive() const
 
 LayoutRect AccessibilityListBoxOption::elementRect() const
 {
-    LayoutRect rect;
-    if (!m_optionElement)
-        return rect;
-
-    HTMLSelectElement* listBoxParentNode = listBoxOptionParentNode();
+    if (!m_node)
+        return { };
+
+    RefPtr listBoxParentNode = listBoxOptionParentNode();
     if (!listBoxParentNode)
-        return rect;
-    
-    RenderElement* listBoxRenderer = listBoxParentNode->renderer();
+        return { };
+
+    auto* listBoxRenderer = dynamicDowncast<RenderListBox>(listBoxParentNode->renderer());
     if (!listBoxRenderer)
-        return rect;
-
-    LayoutRect parentRect = listBoxRenderer->document().axObjectCache()->getOrCreate(listBoxRenderer)->boundingBoxRect();
+        return { };
+
+    WeakPtr cache = listBoxRenderer->document().axObjectCache();
+    RefPtr listbox = cache ? cache->getOrCreate(listBoxRenderer) : nullptr;
+    if (!listbox)
+        return { };
+
+    auto parentRect = listbox->boundingBoxRect();
     int index = listBoxOptionIndex();
     if (index != -1)
-        rect = downcast<RenderListBox>(*listBoxRenderer).itemBoundingBoxRect(parentRect.location(), index);
-
-    return rect;
+        return listBoxRenderer->itemBoundingBoxRect(parentRect.location(), index);
+    return { };
 }
 
 bool AccessibilityListBoxOption::computeAccessibilityIsIgnored() const
 {
-    if (!m_optionElement)
+    if (!m_node)
         return true;
 
     if (accessibilityIsIgnoredByDefault())
@@ -120,57 +113,57 @@ bool AccessibilityListBoxOption::computeAccessibilityIsIgnored() const
     auto* parent = parentObject();
     return parent ? parent->accessibilityIsIgnored() : true;
 }
-    
+
 bool AccessibilityListBoxOption::canSetSelectedAttribute() const
 {
-    if (!is<HTMLOptionElement>(m_optionElement))
+    RefPtr optionElement = dynamicDowncast<HTMLOptionElement>(m_node.get());
+    if (!optionElement)
         return false;
-
-    if (m_optionElement->isDisabledFormControl())
-        return false;
-
-    HTMLSelectElement* selectElement = listBoxOptionParentNode();
-    if (selectElement && selectElement->isDisabledFormControl())
+
+    if (optionElement->isDisabledFormControl())
         return false;
-
-    return true;
+
+    RefPtr selectElement = listBoxOptionParentNode();
+    return !selectElement || !selectElement->isDisabledFormControl();
 }
-    
+
 String AccessibilityListBoxOption::stringValue() const
 {
-    if (!m_optionElement)
-        return String();
+    if (!m_node)
+        return { };
 
     const auto& ariaLabel = getAttribute(aria_labelAttr);
     if (!ariaLabel.isNull())
         return ariaLabel;
 
-    if (RefPtr option = dynamicDowncast<HTMLOptionElement>(*m_optionElement))
+    if (RefPtr option = dynamicDowncast<HTMLOptionElement>(*m_node))
         return option->label();
 
-    if (RefPtr optgroup = dynamicDowncast<HTMLOptGroupElement>(*m_optionElement))
+    if (RefPtr optgroup = dynamicDowncast<HTMLOptGroupElement>(*m_node))
         return optgroup->groupLabelText();
 
-    return String();
+    return { };
 }
 
 Element* AccessibilityListBoxOption::actionElement() const
 {
-    return m_optionElement.get();
+    ASSERT(is<HTMLElement>(m_node.get()));
+    return dynamicDowncast<Element>(m_node.get());
 }
 
 Node* AccessibilityListBoxOption::node() const
 {
-    return m_optionElement.get();
+    return m_node.get();
 }
 
 AccessibilityObject* AccessibilityListBoxOption::parentObject() const
 {
-    HTMLSelectElement* parentNode = listBoxOptionParentNode();
+    auto* parentNode = listBoxOptionParentNode();
     if (!parentNode)
         return nullptr;
-
-    return m_optionElement->document().axObjectCache()->getOrCreate(parentNode);
+
+    auto* cache = m_node->document().axObjectCache();
+    return cache ? cache->getOrCreate(parentNode) : nullptr;
 }
 
 void AccessibilityListBoxOption::setSelected(bool selected)
@@ -193,32 +186,33 @@ void AccessibilityListBoxOption::setSelected(bool selected)
 
 HTMLSelectElement* AccessibilityListBoxOption::listBoxOptionParentNode() const
 {
-    if (!m_optionElement)
+    if (!m_node)
         return nullptr;
 
-    if (RefPtr option = dynamicDowncast<HTMLOptionElement>(*m_optionElement))
+    if (RefPtr option = dynamicDowncast<HTMLOptionElement>(*m_node))
         return option->ownerSelectElement();
 
-    if (RefPtr optgroup = dynamicDowncast<HTMLOptGroupElement>(*m_optionElement))
+    if (RefPtr optgroup = dynamicDowncast<HTMLOptGroupElement>(*m_node))
         return optgroup->ownerSelectElement();
 
     return nullptr;
 }
 
 int AccessibilityListBoxOption::listBoxOptionIndex() const
 {
-    if (!m_optionElement)
+    if (!m_node)
         return -1;
-    
-    HTMLSelectElement* selectElement = listBoxOptionParentNode();
-    if (!selectElement) 
+
+    auto* selectElement = listBoxOptionParentNode();
+    if (!selectElement)
         return -1;
-    
+
     const auto& listItems = selectElement->listItems();
     unsigned length = listItems.size();
-    for (unsigned i = 0; i < length; i++)
-        if (listItems[i] == m_optionElement)
+    for (unsigned i = 0; i < length; i++) {
+        if (listItems[i] == m_node)
             return i;
+    }
 
     return -1;
 }

Source/WebCore/accessibility/AccessibilityListBoxOption.h

@@ -28,14 +28,14 @@
 
 #pragma once
 
-#include "AccessibilityObject.h"
+#include "AccessibilityNodeObject.h"
 
 namespace WebCore {
 
 class HTMLElement;
 class HTMLSelectElement;
 
-class AccessibilityListBoxOption final : public AccessibilityObject {
+class AccessibilityListBoxOption final : public AccessibilityNodeObject {
 public:
     static Ref<AccessibilityListBoxOption> create(HTMLElement&);
     virtual ~AccessibilityListBoxOption();
@@ -64,10 +64,8 @@ class AccessibilityListBoxOption final : public AccessibilityObject {
     IntRect listBoxOptionRect() const;
     AccessibilityObject* listBoxOptionAccessibilityObject(HTMLElement*) const;
     bool computeAccessibilityIsIgnored() const final;
-
-    WeakPtr<HTMLElement, WeakPtrImplWithEventTargetData> m_optionElement;
 };
-    
+
 } // namespace WebCore 
 
 SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY(AccessibilityListBoxOption, isListBoxOption())

Source/WebCore/accessibility/AccessibilityNodeObject.h

@@ -231,7 +231,7 @@ class AccessibilityNodeObject : public AccessibilityObject {
     void setNeedsToUpdateSubtree() override { m_subtreeDirty = true; }
 
     bool isDescendantOfElementType(const HashSet<QualifiedName>&) const;
-
+protected:
     WeakPtr<Node, WeakPtrImplWithEventTargetData> m_node;
 };