AX: aria-activedescendant doesn't work for display:contents elements

https://bugs.webkit.org/show_bug.cgi?id=255648 rdar://problem/108248056 Reviewed by Chris Fleizach. Move `shouldFocusActiveDescendant` and `activeDescendant` from AccessibilityRenderObject to AccessibilityObject since they don't require a renderer. Tested by new test: display-contents-descendant-menu-item.html Also removes two unused functions: - isLinked - hasControlsAttributeSet And rewrites accessibility/mac/aria-tree-activedescendant.html to be a more modern style layout test. * LayoutTests/accessibility/display-contents-descendant-menu-item-expected.txt: Added. * LayoutTests/accessibility/display-contents-descendant-menu-item.html: Added. * LayoutTests/platform/mac-wk1/TestExpectations: Skip new test. * LayoutTests/accessibility/mac/aria-tree-activedescendant-expected.txt: * LayoutTests/accessibility/mac/aria-tree-activedescendant.html: * Source/WebCore/accessibility/AccessibilityImageMapLink.h: * Source/WebCore/accessibility/AccessibilityMediaObject.cpp: (WebCore::AccessibilityMediaObject::hasControlsAttributeSet const): Deleted. * Source/WebCore/accessibility/AccessibilityMediaObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::shouldFocusActiveDescendant const): (WebCore::AccessibilityObject::activeDescendant const): * Source/WebCore/accessibility/AccessibilityObject.h: (WebCore::AccessibilityObject::isLinked const): Deleted. (WebCore::AccessibilityObject::shouldFocusActiveDescendant const): Deleted. * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::observableObject const): (WebCore::AccessibilityRenderObject::expandedTextValue const): (WebCore::AccessibilityRenderObject::shouldFocusActiveDescendant const): Deleted. (WebCore::AccessibilityRenderObject::activeDescendant const): Deleted. (WebCore::AccessibilityRenderObject::isLinked const): Deleted. * Source/WebCore/accessibility/AccessibilityRenderObject.h: Canonical link: https://commits.webkit.org/263163@main
WebKit · Apr 20, 2023 · d488f39 · d488f39
1 parent 8d3513a
commit d488f39
Show file tree

Hide file tree

Showing 12 changed files with 113 additions and 102 deletions.
diff --git a/LayoutTests/accessibility/display-contents-descendant-menu-item-expected.txt b/LayoutTests/accessibility/display-contents-descendant-menu-item-expected.txt
@@ -0,0 +1,10 @@
+This test ensures that the aria-activedescendant of a display:contents menu is considered selected.
+
+PASS: accessibilityController.accessibleElementById('item1').isSelected === true
+PASS: accessibilityController.accessibleElementById('item2').isSelected === false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Menu item 1
+Menu item 2
diff --git a/LayoutTests/accessibility/display-contents-descendant-menu-item.html b/LayoutTests/accessibility/display-contents-descendant-menu-item.html
@@ -0,0 +1,25 @@
+<!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 id="menu" style="display:contents" role="menu" aria-activedescendant="item1">
+   <div role="menuitem" id="item1" tabindex="-1">Menu item 1</div>
+   <div role="menuitem" id="item2" tabindex="-1">Menu item 2</div>
+</div>
+
+<script>
+var output = "This test ensures that the aria-activedescendant of a display:contents menu is considered selected.\n\n";
+
+if (window.accessibilityController) {
+    output += expect("accessibilityController.accessibleElementById('item1').isSelected", "true");
+    output += expect("accessibilityController.accessibleElementById('item2').isSelected", "false");
+    debug(output);
+}
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/accessibility/mac/aria-tree-activedescendant-expected.txt b/LayoutTests/accessibility/mac/aria-tree-activedescendant-expected.txt
@@ -1,7 +1,7 @@
 This tests that the aria-activedescendant attributes works properly.
 
-PASS: axtree.selectedRowAtIndex(0).isEqual(activeDescendant) === true
-PASS: axtree.selectedRowAtIndex(0).isEqual(newActiveDescendant) === true
+PASS: axTree.selectedRowAtIndex(0).isEqual(activeDescendant) === true
+PASS: axTree.selectedRowAtIndex(0).isEqual(newActiveDescendant) === true
 
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/accessibility/mac/aria-tree-activedescendant.html b/LayoutTests/accessibility/mac/aria-tree-activedescendant.html
@@ -1,46 +1,48 @@
 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
 <html>
 <head>
-<script src="../../resources/js-test.js"></script>
 <script src="../../resources/accessibility-helper.js"></script>
+<script src="../../resources/js-test.js"></script>
 </head>
-<body id="body">
+<body>
 
 <ul id="tree0" role="tree" aria-labelledby="treelabel" aria-activedescendant="tree0_item0" tabindex="0">
-<li id="tree0_item0" role="treeitem" aria-level="1" aria-expanded="true">
-<span><span class="expander"></span>Animals</span>
-<ul role="group">
-<li id="tree0_item0_0" role="treeitem" aria-level="2"><span>Birds</span></li>
-<li id="tree0_item0_1" role="treeitem" aria-level="2" aria-expanded="false">
-<span><span class="expander"></span>Cats</span>
-<ul role="group">
-<li id="tree0_item0_1_0" role="treeitem"aria-level="3"><span>Siamese</span></li>
-<li id="tree0_item0_1_1" role="treeitem" aria-level="3"><span>Tabby</span></li>
-</ul>
-</li>
-</ul>
-</li>
+    <li id="tree0_item0" role="treeitem" aria-level="1" aria-expanded="true">
+        <span><span class="expander"></span>Animals</span>
+        <ul role="group">
+            <li id="tree0_item0_0" role="treeitem" aria-level="2"><span>Birds</span></li>
+            <li id="tree0_item0_1" role="treeitem" aria-level="2" aria-expanded="false">
+                <span><span class="expander"></span>Cats</span>
+                <ul role="group">
+                    <li id="tree0_item0_1_0" role="treeitem"aria-level="3"><span>Siamese</span></li>
+                    <li id="tree0_item0_1_1" role="treeitem" aria-level="3"><span>Tabby</span></li>
+                </ul>
+            </li>
+        </ul>
+    </li>
 </ul>
 
 <script>
-    if (window.accessibilityController) {
-        window.jsTestIsAsync = true;
-        var output = "This tests that the aria-activedescendant attributes works properly.\n\n";
-        var axtree, activeDescendant, newActiveDescendant;
-        setTimeout(function() {
-            axtree = accessibilityController.rootElement.childAtIndex(0).childAtIndex(0);
-            activeDescendant = axtree.childAtIndex(0);
-            output += expect("axtree.selectedRowAtIndex(0).isEqual(activeDescendant)", "true");
+var output = "This tests that the aria-activedescendant attributes works properly.\n\n";
+
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+
+    var axTree, activeDescendant, newActiveDescendant;
+    setTimeout(async function() {
+        axTree = accessibilityController.rootElement.childAtIndex(0).childAtIndex(0);
+        activeDescendant = axTree.childAtIndex(0);
+        output += expect("axTree.selectedRowAtIndex(0).isEqual(activeDescendant)", "true");
 
-            newActiveDescendant = axtree.childAtIndex(2);
-            document.getElementById("tree0").setAttribute("aria-activedescendant", "tree0_item0_1");
-            output += expect("axtree.selectedRowAtIndex(0).isEqual(newActiveDescendant)", "true");
+        newActiveDescendant = axTree.childAtIndex(2);
+        document.getElementById("tree0").setAttribute("aria-activedescendant", "tree0_item0_1");
+        output += await expectAsync("axTree.selectedRowAtIndex(0).isEqual(newActiveDescendant)", "true");
 
-            debug(output);
-            tree0.style.visibility = "hidden";
-            finishJSTest();
-        }, 0);
-    }
+        debug(output);
+        tree0.style.visibility = "hidden";
+        finishJSTest();
+    }, 0);
+}
 </script>
 </body>
 </html>
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -1927,6 +1927,7 @@ accessibility/element-reflection-ariaerrormessage.html [ Skip ]
 accessibility/mac/pseudo-element-text-markers.html [ Skip ]
 accessibility/nested-textareas-value-changed-notifications.html [ Skip ]
 accessibility/selected-state-changed-notifications.html [ Skip ]
+accessibility/display-contents-descendant-menu-item.html [ Skip ]
 accessibility/element-line-rects-and-text.html [ Skip ]
 accessibility/mac/line-index-for-textmarker.html [ Skip ]
 accessibility/mac/mathml-elements.html [ Skip ]

diff --git a/Source/WebCore/accessibility/AccessibilityImageMapLink.h b/Source/WebCore/accessibility/AccessibilityImageMapLink.h
@@ -54,7 +54,6 @@ class AccessibilityImageMapLink final : public AccessibilityMockObject {
     Element* actionElement() const override;
     URL url() const override;
     bool isLink() const override { return true; }
-    bool isLinked() const override { return true; }
     String title() const override;
     String description() const override;
     AccessibilityObject* parentObject() const override;

diff --git a/Source/WebCore/accessibility/AccessibilityMediaObject.cpp b/Source/WebCore/accessibility/AccessibilityMediaObject.cpp
@@ -127,15 +127,6 @@ bool AccessibilityMediaObject::press()
     return true;
 }
 
-bool AccessibilityMediaObject::hasControlsAttributeSet() const
-{
-    HTMLMediaElement* element = mediaElement();
-    if (!element)
-        return false;
-
-    return element->controls();
-}
-
 bool AccessibilityMediaObject::isPlaying() const
 {
     HTMLMediaElement* element = mediaElement();

diff --git a/Source/WebCore/accessibility/AccessibilityMediaObject.h b/Source/WebCore/accessibility/AccessibilityMediaObject.h
@@ -42,7 +42,6 @@ class AccessibilityMediaObject final : public AccessibilityRenderObject {
     void enterFullscreen() const;
     void toggleMute();
 
-    bool hasControlsAttributeSet() const;
     String interactiveVideoDuration() const;
     bool isPlaying() const;
     bool isAutoplayEnabled() const;

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -3904,6 +3904,44 @@ AXCoreObject::AccessibilityChildrenVector AccessibilityObject::relatedObjects(AX
     return cache->objectsForIDs(*relatedObjectIDs);
 }
 
+bool AccessibilityObject::shouldFocusActiveDescendant() const
+{
+    switch (ariaRoleAttribute()) {
+    case AccessibilityRole::ApplicationGroup:
+    case AccessibilityRole::ListBox:
+    case AccessibilityRole::Menu:
+    case AccessibilityRole::MenuBar:
+    case AccessibilityRole::RadioGroup:
+    case AccessibilityRole::Row:
+    case AccessibilityRole::PopUpButton:
+    case AccessibilityRole::Meter:
+    case AccessibilityRole::ProgressIndicator:
+    case AccessibilityRole::Toolbar:
+    case AccessibilityRole::Outline:
+    case AccessibilityRole::Tree:
+    case AccessibilityRole::Grid:
+    /* FIXME: replace these with actual roles when they are added to AccessibilityRole
+    composite
+    alert
+    alertdialog
+    status
+    timer
+    */
+        return true;
+    default:
+        return false;
+    }
+}
+
+AccessibilityObject* AccessibilityObject::activeDescendant() const
+{
+    auto activeDescendants = relatedObjects(AXRelationType::ActiveDescendant);
+    ASSERT(activeDescendants.size() <= 1);
+    if (!activeDescendants.isEmpty())
+        return dynamicDowncast<AccessibilityObject>(activeDescendants[0].get());
+    return nullptr;
+}
+
 bool AccessibilityObject::isActiveDescendantOfFocusedContainer() const
 {
     auto containers = activeDescendantOfObjects();

diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h
@@ -213,7 +213,6 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     InsideLink insideLink() const final;
     bool isRequired() const override { return false; }
     bool supportsRequiredAttribute() const override { return false; }
-    virtual bool isLinked() const { return false; }
     bool isExpanded() const override;
     bool isVisible() const override { return true; }
     virtual bool isCollapsed() const { return false; }
@@ -511,8 +510,8 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     void selectedChildren(AccessibilityChildrenVector&) override { }
     void setSelectedChildren(const AccessibilityChildrenVector&) override { }
     AccessibilityChildrenVector visibleChildren() override { return { }; }
-    virtual bool shouldFocusActiveDescendant() const { return false; }
-    AccessibilityObject* activeDescendant() const override { return nullptr; }
+    bool shouldFocusActiveDescendant() const;
+    AccessibilityObject* activeDescendant() const final;
 
     WEBCORE_EXPORT static AccessibilityRole ariaRoleToWebCoreRole(const String&);
     virtual bool hasAttribute(const QualifiedName&) const;

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -2452,44 +2452,6 @@ AXCoreObject* AccessibilityRenderObject::accessibilityHitTest(const IntPoint& po
     return result;
 }
 
-bool AccessibilityRenderObject::shouldFocusActiveDescendant() const
-{
-    switch (ariaRoleAttribute()) {
-    case AccessibilityRole::ApplicationGroup:
-    case AccessibilityRole::ListBox:
-    case AccessibilityRole::Menu:
-    case AccessibilityRole::MenuBar:
-    case AccessibilityRole::RadioGroup:
-    case AccessibilityRole::Row:
-    case AccessibilityRole::PopUpButton:
-    case AccessibilityRole::Meter:
-    case AccessibilityRole::ProgressIndicator:
-    case AccessibilityRole::Toolbar:
-    case AccessibilityRole::Outline:
-    case AccessibilityRole::Tree:
-    case AccessibilityRole::Grid:
-    /* FIXME: replace these with actual roles when they are added to AccessibilityRole
-    composite
-    alert
-    alertdialog
-    status
-    timer
-    */
-        return true;
-    default:
-        return false;
-    }
-}
-
-AccessibilityObject* AccessibilityRenderObject::activeDescendant() const
-{
-    auto activeDescendants = relatedObjects(AXRelationType::ActiveDescendant);
-    ASSERT(activeDescendants.size() <= 1);
-    if (!activeDescendants.isEmpty())
-        return downcast<AccessibilityObject>(activeDescendants[0].get());
-    return nullptr;
-}
-
 bool AccessibilityRenderObject::renderObjectIsObservable(RenderObject& renderer) const
 {
     // AX clients will listen for AXValueChange on a text control.
@@ -2520,7 +2482,7 @@ AccessibilityObject* AccessibilityRenderObject::observableObject() const
                 return cache->getOrCreate(renderer);
         }
     }
-    
+
     return nullptr;
 }
 
@@ -2530,7 +2492,7 @@ String AccessibilityRenderObject::expandedTextValue() const
         if (parent->hasTagName(abbrTag) || parent->hasTagName(acronymTag))
             return parent->getAttribute(titleAttr);
     }
-    
+
     return String();
 }
 
@@ -3128,18 +3090,6 @@ String AccessibilityRenderObject::stringValueForMSAA() const
     return stringValue();
 }
 
-bool AccessibilityRenderObject::isLinked() const
-{
-    if (!isLinkable(*this))
-        return false;
-
-    Element* anchor = anchorElement();
-    if (!is<HTMLAnchorElement>(anchor))
-        return false;
-
-    return !downcast<HTMLAnchorElement>(*anchor).href().isEmpty();
-}
-
 bool AccessibilityRenderObject::hasBoldFont() 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 isOffScreen() const override;
-    bool isLinked() const override;
     bool hasBoldFont() const override;
     bool hasItalicFont() const override;
     bool hasPlainText() const override;
@@ -128,8 +127,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject {
     void addChildren() override;
     bool canHaveChildren() const override;
     void selectedChildren(AccessibilityChildrenVector&) override;
-    bool shouldFocusActiveDescendant() const override;
-    AccessibilityObject* activeDescendant() const override;
 
     VisiblePositionRange visiblePositionRange() const override;
     VisiblePositionRange visiblePositionRangeForLine(unsigned) const override;