Skip to content

Commit

Permalink
Make AXCoreObject::visibleChildren, AXCoreObject::isPresentationalChi…
Browse files Browse the repository at this point in the history
…ldOfAriaRole, 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
  • Loading branch information
twilco committed Apr 17, 2023
1 parent 7a02bc7 commit ee7f5c5
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 73 deletions.
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/accessibility/presentational-children.html
Expand Up @@ -8,6 +8,7 @@
<div role="button" tabindex="0" class="ex"><div>a</div><div>b</div></div>
<div role="checkbox" tabindex="0" class="ex"><div>a</div><div>b</div></div>
<div role="img" tabindex="0" class="ex"><div>a</div><div>b</div></div>
<div role="img" tabindex="0" class="ex" style="display:contents"><div style="display:contents">a</div><div>b</div></div>
<div role="menu">
<div role="menuitemcheckbox" tabindex="0" class="ex"><div>a</div><div>b</div></div>
<div role="menuitemradio" tabindex="0" class="ex"><div>a</div><div>b</div></div>
Expand Down
16 changes: 10 additions & 6 deletions Source/WebCore/accessibility/AccessibilityListBox.cpp
Expand Up @@ -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<RenderListBox>(m_renderer.get()));
auto* listBox = dynamicDowncast<RenderListBox>(m_renderer.get());
if (!listBox)
return { };

if (!childrenInitialized())
addChildren();

unsigned length = m_children.size();
for (unsigned i = 0; i < length; i++) {
if (downcast<RenderListBox>(*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
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityListBox.h
Expand Up @@ -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;

Expand Down
18 changes: 18 additions & 0 deletions Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/AccessibilityNodeObject.h
Expand Up @@ -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;
Expand Down
21 changes: 21 additions & 0 deletions Source/WebCore/accessibility/AccessibilityObject.cpp
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/accessibility/AccessibilityObject.h
Expand Up @@ -361,8 +361,8 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
virtual AccessibilityRole ariaRoleAttribute() const { return AccessibilityRole::Unknown; }
bool hasExplicitGenericRole() const { return ariaRoleAttribute() == AccessibilityRole::Generic; }
bool hasImplicitGenericRole() const { return roleValue() == AccessibilityRole::Generic && !hasExplicitGenericRole(); }
virtual bool isPresentationalChildOfAriaRole() const { return false; }
virtual bool ariaRoleHasPresentationalChildren() const { return false; }
bool isPresentationalChildOfAriaRole() const;
bool ariaRoleHasPresentationalChildren() const;
bool inheritsPresentationalRole() const override { return false; }

// Accessibility Text
Expand Down Expand Up @@ -517,7 +517,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi

void selectedChildren(AccessibilityChildrenVector&) override { }
void setSelectedChildren(const AccessibilityChildrenVector&) override { }
void visibleChildren(AccessibilityChildrenVector&) override { }
AccessibilityChildrenVector visibleChildren() override { return { }; }
virtual bool shouldFocusActiveDescendant() const { return false; }
AccessibilityObject* activeDescendant() const override { return nullptr; }

Expand Down
Expand Up @@ -1250,7 +1250,7 @@ class AXCoreObject : public ThreadSafeRefCounted<AXCoreObject> {
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;
Expand Down
47 changes: 0 additions & 47 deletions Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/accessibility/AccessibilityRenderObject.h
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Expand Up @@ -241,9 +241,10 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& 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()) {
Expand Down
Expand Up @@ -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<AXID>(AXPropertyName::VisibleChildren)); }
AtomString tagName() const override;
const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) override;
void updateChildrenIfNecessary() override;
Expand Down
Expand Up @@ -1003,15 +1003,13 @@ - (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;
}();
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;
Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit ee7f5c5

Please sign in to comment.