From ee7f5c504b704506aa8f20d7b81e1d281fdeb1d9 Mon Sep 17 00:00:00 2001 From: Tyler Wilcock Date: Mon, 17 Apr 2023 09:47:32 -0700 Subject: [PATCH] Make AXCoreObject::visibleChildren, AXCoreObject::isPresentationalChildOfAriaRole, and AXCoreObject::ariaRoleHasPresentationalChildren work for display:contents elements https://bugs.webkit.org/show_bug.cgi?id=255371 rdar://problem/107962942 Reviewed by Chris Fleizach and Andres Gonzalez. None of these methods inherently require a renderer, but are still only implemented for AccessibilityRenderObject, meaning they won't work for display:contents elements. This patch fixes this by moving the implementations to the appropriate base class for each method. Other significant changes worth mentioning: 1. `visibleChildren` for ARIA listboxes had a bug where it selected children that were `isOffScreen` rather than `!isOffScreen`. 2. NSAccessibilityVisibleChildrenAttribute was removed as a supported attribute for menus and menu bars since we always return nil for this combination of role and attribute. * LayoutTests/accessibility/presentational-children-expected.txt: * LayoutTests/accessibility/presentational-children.html: * Source/WebCore/accessibility/AccessibilityListBox.cpp: (WebCore::AccessibilityListBox::visibleChildren): * Source/WebCore/accessibility/AccessibilityListBox.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::visibleChildren): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::ariaRoleHasPresentationalChildren const): (WebCore::AccessibilityObject::isPresentationalChildOfAriaRole const): * Source/WebCore/accessibility/AccessibilityObject.h: (WebCore::AccessibilityObject::isPresentationalChildOfAriaRole const): Deleted. (WebCore::AccessibilityObject::ariaRoleHasPresentationalChildren const): Deleted. * Source/WebCore/accessibility/AccessibilityObjectInterface.h: * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::isPresentationalChildOfAriaRole const): Deleted. (WebCore::AccessibilityRenderObject::ariaRoleHasPresentationalChildren const): Deleted. (WebCore::AccessibilityRenderObject::ariaListboxVisibleChildren): Deleted. (WebCore::AccessibilityRenderObject::visibleChildren): Deleted. * Source/WebCore/accessibility/AccessibilityRenderObject.h: * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp: (WebCore::AXIsolatedObject::initializeProperties): * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h: * Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm: (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]): Canonical link: https://commits.webkit.org/263025@main --- .../presentational-children-expected.txt | 1 + .../presentational-children.html | 1 + .../accessibility/AccessibilityListBox.cpp | 16 ++++--- .../accessibility/AccessibilityListBox.h | 2 +- .../accessibility/AccessibilityNodeObject.cpp | 18 +++++++ .../accessibility/AccessibilityNodeObject.h | 1 + .../accessibility/AccessibilityObject.cpp | 21 +++++++++ .../accessibility/AccessibilityObject.h | 6 +-- .../AccessibilityObjectInterface.h | 2 +- .../AccessibilityRenderObject.cpp | 47 ------------------- .../accessibility/AccessibilityRenderObject.h | 4 -- .../isolatedtree/AXIsolatedObject.cpp | 7 +-- .../isolatedtree/AXIsolatedObject.h | 2 +- .../mac/WebAccessibilityObjectWrapperMac.mm | 9 +--- 14 files changed, 64 insertions(+), 73 deletions(-) diff --git a/LayoutTests/accessibility/presentational-children-expected.txt b/LayoutTests/accessibility/presentational-children-expected.txt index f1f90777fb63..5c67a9bbb4f4 100644 --- a/LayoutTests/accessibility/presentational-children-expected.txt +++ b/LayoutTests/accessibility/presentational-children-expected.txt @@ -6,6 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE button childrenCount: 0 checkbox childrenCount: 0 img childrenCount: 0 +img childrenCount: 0 menuitemcheckbox childrenCount: 0 menuitemradio childrenCount: 0 meter childrenCount: 0 diff --git a/LayoutTests/accessibility/presentational-children.html b/LayoutTests/accessibility/presentational-children.html index bee3d03303ef..d22193161f1b 100644 --- a/LayoutTests/accessibility/presentational-children.html +++ b/LayoutTests/accessibility/presentational-children.html @@ -8,6 +8,7 @@
a
b
+
a
b
a
b
diff --git a/Source/WebCore/accessibility/AccessibilityListBox.cpp b/Source/WebCore/accessibility/AccessibilityListBox.cpp index dfb2974214d3..a8bfb0833839 100644 --- a/Source/WebCore/accessibility/AccessibilityListBox.cpp +++ b/Source/WebCore/accessibility/AccessibilityListBox.cpp @@ -114,18 +114,22 @@ void AccessibilityListBox::selectedChildren(AccessibilityChildrenVector& result) } } -void AccessibilityListBox::visibleChildren(AccessibilityChildrenVector& result) +AXCoreObject::AccessibilityChildrenVector AccessibilityListBox::visibleChildren() { - ASSERT(result.isEmpty()); - + ASSERT(!m_renderer || is(m_renderer.get())); + auto* listBox = dynamicDowncast(m_renderer.get()); + if (!listBox) + return { }; + if (!childrenInitialized()) addChildren(); - unsigned length = m_children.size(); - for (unsigned i = 0; i < length; i++) { - if (downcast(*m_renderer).listIndexIsVisible(i)) + AccessibilityChildrenVector result; + for (unsigned i = 0; i < m_children.size(); i++) { + if (listBox->listIndexIsVisible(i)) result.append(m_children[i]); } + return result; } AccessibilityObject* AccessibilityListBox::listBoxOptionAccessibilityObject(HTMLElement* element) const diff --git a/Source/WebCore/accessibility/AccessibilityListBox.h b/Source/WebCore/accessibility/AccessibilityListBox.h index bb9760c7a1c6..f89f6020af23 100644 --- a/Source/WebCore/accessibility/AccessibilityListBox.h +++ b/Source/WebCore/accessibility/AccessibilityListBox.h @@ -42,7 +42,7 @@ class AccessibilityListBox final : public AccessibilityRenderObject { AccessibilityRole roleValue() const override { return AccessibilityRole::ListBox; } void selectedChildren(AccessibilityChildrenVector&) override; - void visibleChildren(AccessibilityChildrenVector&) override; + AccessibilityChildrenVector visibleChildren() final; void addChildren() override; diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp index 520400ebeee5..75f9abecc1ca 100644 --- a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp @@ -546,6 +546,24 @@ bool AccessibilityNodeObject::canHaveChildren() const } } +AXCoreObject::AccessibilityChildrenVector AccessibilityNodeObject::visibleChildren() +{ + // Only listboxes are asked for their visible children. + // Native list boxes would be AccessibilityListBoxes, so only check for aria list boxes. + if (ariaRoleAttribute() != AccessibilityRole::ListBox) + return { }; + + if (!childrenInitialized()) + addChildren(); + + AccessibilityChildrenVector result; + for (const auto& child : children()) { + if (!child->isOffScreen()) + result.append(child); + } + return result; +} + bool AccessibilityNodeObject::computeAccessibilityIsIgnored() const { #ifndef NDEBUG diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h index 3d989fb8159c..5e3efe5421b8 100644 --- a/Source/WebCore/accessibility/AccessibilityNodeObject.h +++ b/Source/WebCore/accessibility/AccessibilityNodeObject.h @@ -156,6 +156,7 @@ class AccessibilityNodeObject : public AccessibilityObject { void clearChildren() override; void updateChildrenIfNecessary() override; bool canHaveChildren() const override; + AccessibilityChildrenVector visibleChildren() override; bool isDescendantOfBarrenParent() const override; void updateOwnedChildren(); AccessibilityObject* ownerParentObject() const; diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp index fa508302fa14..6d21d24360fe 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityObject.cpp @@ -3907,6 +3907,27 @@ bool AccessibilityObject::isActiveDescendantOfFocusedContainer() const return false; } +bool AccessibilityObject::ariaRoleHasPresentationalChildren() const +{ + switch (ariaRoleAttribute()) { + case AccessibilityRole::Button: + case AccessibilityRole::Slider: + case AccessibilityRole::Image: + case AccessibilityRole::ProgressIndicator: + case AccessibilityRole::SpinButton: + return true; + default: + return false; + } +} + +bool AccessibilityObject::isPresentationalChildOfAriaRole() const +{ + return Accessibility::findAncestor(*this, false, [] (const auto& ancestor) { + return ancestor.ariaRoleHasPresentationalChildren(); + }); +} + void AccessibilityObject::setIsIgnoredFromParentDataForChild(AccessibilityObject* child) { if (!child) diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h index b6d5d52938a2..285e73bf4a4d 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.h +++ b/Source/WebCore/accessibility/AccessibilityObject.h @@ -361,8 +361,8 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr { bool canHaveSelectedChildren() const; virtual void selectedChildren(AccessibilityChildrenVector&) = 0; virtual void setSelectedChildren(const AccessibilityChildrenVector&) = 0; - virtual void visibleChildren(AccessibilityChildrenVector&) = 0; + virtual AccessibilityChildrenVector visibleChildren() = 0; AccessibilityChildrenVector tabChildren(); virtual AXCoreObject* activeDescendant() const = 0; bool isDescendantOfObject(const AXCoreObject*) const; diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp index e6085325caf9..2493aaef812b 100644 --- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp +++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp @@ -2708,31 +2708,6 @@ bool AccessibilityRenderObject::inheritsPresentationalRole() const return false; } -bool AccessibilityRenderObject::isPresentationalChildOfAriaRole() const -{ - // Walk the parent chain looking for a parent that has presentational children - AccessibilityObject* parent; - for (parent = parentObject(); parent && !parent->ariaRoleHasPresentationalChildren(); parent = parent->parentObject()) - { } - - return parent; -} - -bool AccessibilityRenderObject::ariaRoleHasPresentationalChildren() const -{ - switch (m_ariaRole) { - case AccessibilityRole::Button: - case AccessibilityRole::Slider: - case AccessibilityRole::Image: - case AccessibilityRole::ProgressIndicator: - case AccessibilityRole::SpinButton: - // case SeparatorRole: - return true; - default: - return false; - } -} - void AccessibilityRenderObject::addImageMapChildren() { RenderBoxModelObject* cssBox = renderBoxModelObject(); @@ -3133,28 +3108,6 @@ void AccessibilityRenderObject::selectedChildren(AccessibilityChildrenVector& re } } -void AccessibilityRenderObject::ariaListboxVisibleChildren(AccessibilityChildrenVector& result) -{ - if (!childrenInitialized()) - addChildren(); - - for (const auto& child : children()) { - if (child->isOffScreen()) - result.append(child); - } -} - -void AccessibilityRenderObject::visibleChildren(AccessibilityChildrenVector& result) -{ - ASSERT(result.isEmpty()); - - // Only listboxes are asked for their visible children. - // Native list boxes would be AccessibilityListBoxes, so only check for aria list boxes. - if (ariaRoleAttribute() != AccessibilityRole::ListBox) - return; - return ariaListboxVisibleChildren(result); -} - void AccessibilityRenderObject::setAccessibleName(const AtomString& name) { // Setting the accessible name can store the value in the DOM diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.h b/Source/WebCore/accessibility/AccessibilityRenderObject.h index 7a0159af9954..198d28331a4f 100644 --- a/Source/WebCore/accessibility/AccessibilityRenderObject.h +++ b/Source/WebCore/accessibility/AccessibilityRenderObject.h @@ -88,8 +88,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject { AccessibilityObject* titleUIElement() const override; bool supportsARIAOwns() const override; - bool isPresentationalChildOfAriaRole() const override; - bool ariaRoleHasPresentationalChildren() const override; // Should be called on the root accessibility object to kick off a hit test. AXCoreObject* accessibilityHitTest(const IntPoint&) const override; @@ -132,7 +130,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject { void addChildren() override; bool canHaveChildren() const override; void selectedChildren(AccessibilityChildrenVector&) override; - void visibleChildren(AccessibilityChildrenVector&) override; bool shouldFocusActiveDescendant() const override; AccessibilityObject* activeDescendant() const override; @@ -193,7 +190,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject { private: bool isAccessibilityRenderObject() const final { return true; } void ariaListboxSelectedChildren(AccessibilityChildrenVector&); - void ariaListboxVisibleChildren(AccessibilityChildrenVector&); bool isAllowedChildOfTree() const; String positionalDescriptionForMSAA() const; PlainTextRange documentBasedSelectedTextRange() const; diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp index 7ffc83ba9d6c..14dac092a4c3 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp @@ -241,9 +241,10 @@ void AXIsolatedObject::initializeProperties(const Ref& axOb if (object.isImage()) setProperty(AXPropertyName::EmbeddedImageDescription, object.embeddedImageDescription().isolatedCopy()); - AccessibilityChildrenVector visibleChildren; - object.visibleChildren(visibleChildren); - setObjectVectorProperty(AXPropertyName::VisibleChildren, visibleChildren); + // On macOS, we only advertise support for the visible children attribute for listboxes. + if (object.isListBox()) + setObjectVectorProperty(AXPropertyName::VisibleChildren, object.visibleChildren()); + setObjectVectorProperty(AXPropertyName::LinkedObjects, object.linkedObjects()); if (object.isSpinButton()) { diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h index 78ddc9ddaad4..5883c7b56dd0 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h @@ -294,7 +294,7 @@ class AXIsolatedObject final : public AXCoreObject { String language() const override { return stringAttributeValue(AXPropertyName::Language); } void selectedChildren(AccessibilityChildrenVector& children) override { fillChildrenVectorForProperty(AXPropertyName::SelectedChildren, children); } void setSelectedChildren(const AccessibilityChildrenVector&) override; - void visibleChildren(AccessibilityChildrenVector& children) override { fillChildrenVectorForProperty(AXPropertyName::VisibleChildren, children); } + AccessibilityChildrenVector visibleChildren() override { return tree()->objectsForIDs(vectorAttributeValue(AXPropertyName::VisibleChildren)); } AtomString tagName() const override; const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) override; void updateChildrenIfNecessary() override; diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm index 29de8c279ceb..59992d0f0ced 100644 --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm +++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm @@ -1003,7 +1003,6 @@ - (NSArray*)accessibilityAttributeNames static NeverDestroyed menuBarAttrs = [] { auto tempArray = adoptNS([[NSMutableArray alloc] initWithArray:commonMenuAttrs.get().get()]); [tempArray addObject:NSAccessibilitySelectedChildrenAttribute]; - [tempArray addObject:NSAccessibilityVisibleChildrenAttribute]; [tempArray addObject:NSAccessibilityTitleUIElementAttribute]; [tempArray addObject:NSAccessibilityOrientationAttribute]; return tempArray; @@ -1011,7 +1010,6 @@ - (NSArray*)accessibilityAttributeNames static NeverDestroyed menuAttrs = [] { auto tempArray = adoptNS([[NSMutableArray alloc] initWithArray:commonMenuAttrs.get().get()]); [tempArray addObject:NSAccessibilitySelectedChildrenAttribute]; - [tempArray addObject:NSAccessibilityVisibleChildrenAttribute]; [tempArray addObject:NSAccessibilityTitleUIElementAttribute]; [tempArray addObject:NSAccessibilityOrientationAttribute]; return tempArray; @@ -1615,11 +1613,8 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName } if ([attributeName isEqualToString: NSAccessibilityVisibleChildrenAttribute]) { - if (backingObject->isListBox()) { - AccessibilityObject::AccessibilityChildrenVector visibleChildrenCopy; - backingObject->visibleChildren(visibleChildrenCopy); - return makeNSArray(visibleChildrenCopy); - } + if (backingObject->isListBox()) + return makeNSArray(backingObject->visibleChildren()); if (backingObject->isList()) return [self accessibilityAttributeValue:NSAccessibilityChildrenAttribute];