AX: Properly expose lists that have display:contents list items

https://bugs.webkit.org/show_bug.cgi?id=255325 rdar://problem/107924637 Reviewed by Andres Gonzalez. <div role="list" id="list"> <li style="display:contents">One</li> <li style="display:contents">Two</li> </div> Prior to this patch, we didn't expose this as a list because `AccessibilityList::determineAccessibilityRole()` required renderers for its children which is not valid in the case of display:contents. This patch also features two other display:contents improvements: 1. display:contents elements will now be properly ignored when visibility:hidden 2. AXCoreObject::isLoaded will now do the right thing for display:contents elements * LayoutTests/accessibility/display-contents-list-expected.txt: Added. * LayoutTests/accessibility/display-contents-list.html: Added. * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::createFromNode): (WebCore::AXObjectCache::getOrCreate): * Source/WebCore/accessibility/AccessibilityList.cpp: (WebCore::AccessibilityList::isUnorderedList const): (WebCore::AccessibilityList::isOrderedList const): (WebCore::AccessibilityList::isDescriptionList const): (WebCore::AccessibilityList::childHasPseudoVisibleListItemMarkers): (WebCore::AccessibilityList::determineAccessibilityRole): * Source/WebCore/accessibility/AccessibilityList.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::create): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::isLoaded const): (WebCore::AccessibilityObject::defaultObjectInclusion const): * Source/WebCore/accessibility/AccessibilityObject.h: * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::defaultObjectInclusion const): (WebCore::AccessibilityRenderObject::isLoaded const): Deleted. * Source/WebCore/accessibility/AccessibilityRenderObject.h: Canonical link: https://commits.webkit.org/262889@main
WebKit · Apr 12, 2023 · f49113b · f49113b
1 parent ab2b21f
commit f49113b
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 67 deletions.
diff --git a/LayoutTests/accessibility/display-contents-list-expected.txt b/LayoutTests/accessibility/display-contents-list-expected.txt
@@ -0,0 +1,14 @@
+This test ensures we properly expose lists and list items with display:contents.
+
+PASS: accessibilityController.accessibleElementById('list').role.includes('List') === true
+Adding display:contents and visibility:hidden to #list.
+PASS: #list is ignored.
+Making #list visibility:visible.
+PASS: accessibilityController.accessibleElementById('list').role.includes('List') === true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+One
+ Two
+
diff --git a/LayoutTests/accessibility/display-contents-list.html b/LayoutTests/accessibility/display-contents-list.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
+</head>
+<body>
+
+<div role="list" id="list">
+    <li style="display:contents">One</li>
+    <li style="display:contents">Two</li>
+</div>
+
+<script>
+var output = "This test ensures we properly expose lists and list items with display:contents.\n\n";
+
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+    output += expect("accessibilityController.accessibleElementById('list').role.includes('List')", "true");
+
+    output += "Adding display:contents and visibility:hidden to #list.\n"
+    // Ensure visibility:hidden works for display:contents elements.
+    document.getElementById("list").style.display = "contents";
+    document.getElementById("list").style.visibility = "hidden";
+
+    setTimeout(async function() {
+        await waitFor(() => !accessibilityController.accessibleElementById("list"));
+        output += "PASS: #list is ignored.\n"
+
+        output += "Making #list visibility:visible.\n"
+        document.getElementById("list").style.visibility = "visible";
+        await waitFor(() => accessibilityController.accessibleElementById("list"));
+        output += expect("accessibilityController.accessibleElementById('list').role.includes('List')", "true");
+
+        debug(output);
+        finishJSTest();
+    }, 0);
+}
+</script>
+</body>
+</html>
+
diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -671,7 +671,7 @@ Ref<AccessibilityObject> AXObjectCache::createObjectFromRenderer(RenderObject* r
     return AccessibilityRenderObject::create(renderer);
 }
 
-static Ref<AccessibilityObject> createFromNode(Node* node)
+static Ref<AccessibilityObject> createFromNode(Node& node)
 {
     return AccessibilityNodeObject::create(node);
 }
@@ -767,20 +767,20 @@ AccessibilityObject* AXObjectCache::getOrCreate(Node* node)
     if (inCanvasSubtree)
         node->document().updateStyleIfNeeded();
 
-    RefPtr<AccessibilityObject> newObj = createFromNode(node);
+    RefPtr<AccessibilityObject> newObject = createFromNode(*node);
 
     // Will crash later if we have two objects for the same node.
     ASSERT(!get(node));
 
-    cacheAndInitializeWrapper(newObj.get(), node);
+    cacheAndInitializeWrapper(newObject.get(), node);
     // Compute the object's initial ignored status.
-    newObj->recomputeIsIgnored();
+    newObject->recomputeIsIgnored();
     // Sometimes asking accessibilityIsIgnored() will cause the newObject to be deallocated, and then
     // it will disappear when this function is finished, leading to a use-after-free.
-    if (newObj->isDetached())
+    if (newObject->isDetached())
         return nullptr;
 
-    return newObj.get();
+    return newObject.get();
 }
 
 AccessibilityObject* AXObjectCache::getOrCreate(RenderObject* renderer)

diff --git a/Source/WebCore/accessibility/AccessibilityList.cpp b/Source/WebCore/accessibility/AccessibilityList.cpp
@@ -60,65 +60,56 @@ bool AccessibilityList::computeAccessibilityIsIgnored() const
 
 bool AccessibilityList::isUnorderedList() const
 {
-    if (!m_renderer)
-        return false;
-
-    Node* node = m_renderer->node();
-
     // The ARIA spec says the "list" role is supposed to mimic a UL or OL tag.
     // Since it can't be both, it's probably OK to say that it's an un-ordered list.
     // On the Mac, there's no distinction to the client.
     if (ariaRoleAttribute() == AccessibilityRole::List)
         return true;
-
+
+    auto* node = this->node();
     return node && node->hasTagName(ulTag);
 }
 
 bool AccessibilityList::isOrderedList() const
 {
-    if (!m_renderer)
-        return false;
-
     // ARIA says a directory is like a static table of contents, which sounds like an ordered list.
     if (ariaRoleAttribute() == AccessibilityRole::Directory)
         return true;
 
-    Node* node = m_renderer->node();
-    return node && node->hasTagName(olTag);    
+    auto* node = this->node();
+    return node && node->hasTagName(olTag);
 }
 
 bool AccessibilityList::isDescriptionList() const
 {
-    if (!m_renderer)
-        return false;
-
-    Node* node = m_renderer->node();
+    auto* node = this->node();
     return node && node->hasTagName(dlTag);
 }
 
-bool AccessibilityList::childHasPseudoVisibleListItemMarkers(RenderObject* listItem)
+bool AccessibilityList::childHasPseudoVisibleListItemMarkers(Node* node)
 {
     // Check if the list item has a pseudo-element that should be accessible (e.g. an image or text)
-    Element* listItemElement = downcast<Element>(listItem->node());
-    if (!listItemElement || !listItemElement->beforePseudoElement())
+    auto* element = dynamicDowncast<Element>(node);
+    auto* beforePseudo = element ? element->beforePseudoElement() : nullptr;
+    if (!beforePseudo)
         return false;
 
-    AccessibilityObject* axObj = axObjectCache()->getOrCreate(listItemElement->beforePseudoElement()->renderer());
-    if (!axObj)
+    RefPtr axBeforePseudo = axObjectCache()->getOrCreate(beforePseudo->renderer());
+    if (!axBeforePseudo)
         return false;
 
-    if (!axObj->accessibilityIsIgnored())
+    if (!axBeforePseudo->accessibilityIsIgnored())
         return true;
 
-    for (const auto& child : axObj->children()) {
+    for (const auto& child : axBeforePseudo->children()) {
         if (!child->accessibilityIsIgnored())
             return true;
     }
 
     // Platforms which expose rendered text content through the parent element will treat
     // those renderers as "ignored" objects.
 #if USE(ATSPI)
-    String text = axObj->textUnderElement();
+    String text = axBeforePseudo->textUnderElement();
     return !text.isEmpty() && !text.isAllSpecialCharacters<isHTMLSpace>();
 #else
     return false;
@@ -159,21 +150,17 @@ AccessibilityRole AccessibilityList::determineAccessibilityRole()
         if (axChild && axChild->ariaRoleAttribute() == AccessibilityRole::ListItem)
             listItemCount++;
         else if (child->roleValue() == AccessibilityRole::ListItem) {
-            RenderObject* listItem = child->renderer();
-            if (!listItem)
-                continue;
-
             // Rendered list items always count.
-            if (listItem->isListItem()) {
-                if (!hasVisibleMarkers && (listItem->style().listStyleType().type != ListStyleType::Type::None || listItem->style().listStyleImage() || childHasPseudoVisibleListItemMarkers(listItem)))
+            if (auto* childRenderer = child->renderer(); childRenderer && childRenderer->isListItem()) {
+                if (!hasVisibleMarkers && (childRenderer->style().listStyleType().type != ListStyleType::Type::None || childRenderer->style().listStyleImage() || childHasPseudoVisibleListItemMarkers(childRenderer->node())))
                     hasVisibleMarkers = true;
                 listItemCount++;
-            } else if (listItem->node() && listItem->node()->hasTagName(liTag)) {
+            } else if (child->node() && child->node()->hasTagName(liTag)) {
                 // Inline elements that are in a list with an explicit role should also count.
                 if (m_ariaRole == AccessibilityRole::List)
                     listItemCount++;
 
-                if (childHasPseudoVisibleListItemMarkers(listItem)) {
+                if (childHasPseudoVisibleListItemMarkers(child->node())) {
                     hasVisibleMarkers = true;
                     listItemCount++;
                 }
@@ -188,7 +175,7 @@ AccessibilityRole AccessibilityList::determineAccessibilityRole()
             role = AccessibilityRole::ApplicationGroup;
     } else if (!hasVisibleMarkers) {
         // http://webkit.org/b/193382 lists inside of navigation hierarchies should still be considered lists.
-        if (Accessibility::findAncestor<AXCoreObject>(*this, false, [] (auto& object) { return object.roleValue() == AccessibilityRole::LandmarkNavigation; }))
+        if (Accessibility::findAncestor<AccessibilityObject>(*this, false, [] (auto& object) { return object.roleValue() == AccessibilityRole::LandmarkNavigation; }))
             role = AccessibilityRole::List;
         else
             role = AccessibilityRole::Group;

diff --git a/Source/WebCore/accessibility/AccessibilityList.h b/Source/WebCore/accessibility/AccessibilityList.h
@@ -47,7 +47,7 @@ class AccessibilityList final : public AccessibilityRenderObject {
 
     bool computeAccessibilityIsIgnored() const override;
     AccessibilityRole determineAccessibilityRole() override;
-    bool childHasPseudoVisibleListItemMarkers(RenderObject*);
+    bool childHasPseudoVisibleListItemMarkers(Node*);
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
@@ -108,9 +108,9 @@ void AccessibilityNodeObject::init()
     m_role = determineAccessibilityRole();
 }
 
-Ref<AccessibilityNodeObject> AccessibilityNodeObject::create(Node* node)
+Ref<AccessibilityNodeObject> AccessibilityNodeObject::create(Node& node)
 {
-    return adoptRef(*new AccessibilityNodeObject(node));
+    return adoptRef(*new AccessibilityNodeObject(&node));
 }
 
 void AccessibilityNodeObject::detachRemoteParts(AccessibilityDetachmentType detachmentType)

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h
@@ -46,7 +46,7 @@ enum MouseButtonListenerResultFilter {
 
 class AccessibilityNodeObject : public AccessibilityObject {
 public:
-    static Ref<AccessibilityNodeObject> create(Node*);
+    static Ref<AccessibilityNodeObject> create(Node&);
     virtual ~AccessibilityNodeObject();
 
     void init() override;

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -2700,7 +2700,13 @@ String AccessibilityObject::linkRelValue() const
 {
     return getAttribute(relAttr);
 }
-
+
+bool AccessibilityObject::isLoaded() const
+{
+    auto* document = this->document();
+    return document && !document->parser();
+}
+
 bool AccessibilityObject::isInlineText() const
 {
     return is<RenderInline>(renderer());
@@ -3578,13 +3584,20 @@ String AccessibilityObject::validationMessage() const
 
 AccessibilityObjectInclusion AccessibilityObject::defaultObjectInclusion() const
 {
-    bool useParentData = !m_isIgnoredFromParentData.isNull();
+    if (auto* style = this->style()) {
+        if (style->effectiveInert())
+            return AccessibilityObjectInclusion::IgnoreObject;
+        if (style->visibility() != Visibility::Visible) {
+            // aria-hidden is meant to override visibility as the determinant in AX hierarchy inclusion.
+            if (equalLettersIgnoringASCIICase(getAttribute(aria_hiddenAttr), "false"_s))
+                return AccessibilityObjectInclusion::DefaultBehavior;
 
-    if (useParentData ? m_isIgnoredFromParentData.isAXHidden : isAXHidden())
-        return AccessibilityObjectInclusion::IgnoreObject;
+            return AccessibilityObjectInclusion::IgnoreObject;
+        }
+    }
 
-    auto* style = this->style();
-    if (style && style->effectiveInert())
+    bool useParentData = !m_isIgnoredFromParentData.isNull();
+    if (useParentData ? m_isIgnoredFromParentData.isAXHidden : isAXHidden())
         return AccessibilityObjectInclusion::IgnoreObject;
 
     if (useParentData ? m_isIgnoredFromParentData.isPresentationalChildOfAriaRole : isPresentationalChildOfAriaRole())

diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h
@@ -211,7 +211,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     bool isFocused() const override { return false; }
     virtual bool isHovered() const { return false; }
     bool isIndeterminate() const override { return false; }
-    bool isLoaded() const override { return false; }
+    bool isLoaded() const final;
     bool isMultiSelectable() const override { return false; }
     bool isOffScreen() const override { return false; }
     bool isPressed() const override { return false; }

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -1235,20 +1235,9 @@ static AccessibilityObjectInclusion objectInclusionFromAltText(const String& alt
 
 AccessibilityObjectInclusion AccessibilityRenderObject::defaultObjectInclusion() const
 {
-    // The following cases can apply to any element that's a subclass of AccessibilityRenderObject.
-
     if (!m_renderer)
         return AccessibilityObjectInclusion::IgnoreObject;
-
-    if (m_renderer->style().visibility() != Visibility::Visible) {
-        // aria-hidden is meant to override visibility as the determinant in AX hierarchy inclusion.
-        if (equalLettersIgnoringASCIICase(getAttribute(aria_hiddenAttr), "false"_s))
-            return AccessibilityObjectInclusion::DefaultBehavior;
-
-        return AccessibilityObjectInclusion::IgnoreObject;
-    }
-
-    return AccessibilityObject::defaultObjectInclusion();
+    return AccessibilityNodeObject::defaultObjectInclusion();
 }
 
 static bool webAreaIsPresentational(RenderObject* renderer)
@@ -1534,11 +1523,6 @@ bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
     return true;
 }
 
-bool AccessibilityRenderObject::isLoaded() const
-{
-    return m_renderer ? !m_renderer->document().parser() : false;
-}
-
 double AccessibilityRenderObject::loadingProgress() const
 {
     if (!m_renderer)

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.h b/Source/WebCore/accessibility/AccessibilityRenderObject.h
@@ -58,7 +58,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject {
 
     bool isAttachment() const override;
     bool isSelected() const override;
-    bool isLoaded() const override;
     bool isOffScreen() const override;
     bool isUnvisited() const override;
     bool isVisited() const override;