Skip to content

Commit

Permalink
AX: AXObjectCache::dirtyIsolatedTreeRelations is called whether relat…
Browse files Browse the repository at this point in the history 
…ions are modified or not.

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

Reviewed by Tyler Wilcock.

This causes unnecessary updates to the isolated tree. This patch fixes the issue by making the addRelation and removeRelation method to return a boolean indicating whether the relationships were actually changed.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::updateLabelFor):
(WebCore::AXObjectCache::updateLabeledBy):
(WebCore::AXObjectCache::addRelation):
(WebCore::AXObjectCache::removeAllRelations):
(WebCore::AXObjectCache::removeRelation):
(WebCore::AXObjectCache::updateRelationsForTree):
(WebCore::AXObjectCache::addLabelForRelation):
(WebCore::AXObjectCache::updateRelations):
(WebCore::AXObjectCache::removeRelations): Renamed.
(WebCore::AXObjectCache::addRelations): Renamed.
* Source/WebCore/accessibility/AXObjectCache.h:

Canonical link: https://commits.webkit.org/273341@main
  • Loading branch information
@AndresGonzalezApple
AndresGonzalezApple committed Jan 23, 2024
1 parent d7336d4 commit 36de400
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 53 deletions.
108 changes: 61 additions & 47 deletions Source/WebCore/accessibility/AXObjectCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ void AXObjectCache::remove(AXID axID)
tree->removeNode(*object);
#endif

removeRelations(axID);
removeAllRelations(axID);
object->detach(AccessibilityDetachmentType::ElementDestroyed);

m_idsInUse.remove(axID);
Expand Down Expand Up @@ -2650,7 +2650,7 @@ void AXObjectCache::handleLabelChanged(AccessibilityObject* object)

void AXObjectCache::updateLabelFor(HTMLLabelElement& label)
{
removeRelations(label, AXRelationType::LabelFor);
removeRelation(label, AXRelationType::LabelFor);
addLabelForRelation(label);
}

Expand All @@ -2659,9 +2659,10 @@ void AXObjectCache::updateLabeledBy(Element* element)
if (!element)
return;

removeRelations(*element, AXRelationType::LabeledBy);
addRelations(*element, aria_labelledbyAttr);
dirtyIsolatedTreeRelations();
bool changedRelation = removeRelation(*element, AXRelationType::LabeledBy);
changedRelation |= addRelation(*element, aria_labelledbyAttr);
if (changedRelation)
dirtyIsolatedTreeRelations();
}

void AXObjectCache::dirtyIsolatedTreeRelations()
Expand Down Expand Up @@ -4672,25 +4673,25 @@ static bool validRelation(void* origin, void* target, AXRelationType relationTyp
return origin != target || relationType == AXRelationType::LabeledBy;
}

void AXObjectCache::addRelation(Element* origin, Element* target, AXRelationType relationType)
bool AXObjectCache::addRelation(Element* origin, Element* target, AXRelationType relationType)
{
AXTRACE("AXObjectCache::addRelation"_s);
AXLOG(makeString("origin: ", origin->debugDescription(), " target: ", target->debugDescription(), " relationType ", String::number(static_cast<uint8_t>(relationType))));

if (!validRelation(origin, target, relationType)) {
ASSERT_NOT_REACHED();
return;
return false;
}

if (relationType == AXRelationType::LabelFor) {
// Add a LabelFor relation if the target doesn't have an ARIA label which should take precedence.
if (target->hasAttributeWithoutSynchronization(aria_labelAttr)
|| target->hasAttributeWithoutSynchronization(aria_labelledbyAttr)
|| target->hasAttributeWithoutSynchronization(aria_labeledbyAttr))
return;
return false;
}

addRelation(getOrCreate(origin, IsPartOfRelation::Yes), getOrCreate(target, IsPartOfRelation::Yes), relationType);
return addRelation(getOrCreate(origin, IsPartOfRelation::Yes), getOrCreate(target, IsPartOfRelation::Yes), relationType);
}

static bool canHaveRelations(Element& element)
Expand All @@ -4716,18 +4717,18 @@ static bool relationCausesCycle(AccessibilityObject* origin, AccessibilityObject
return false;
}

void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject* target, AXRelationType relationType, AddSymmetricRelation addSymmetricRelation)
bool AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject* target, AXRelationType relationType, AddSymmetricRelation addSymmetricRelation)
{
AXTRACE("AXObjectCache::addRelation"_s);
AXLOG(origin);
AXLOG(target);
AXLOG(relationType);

if (!validRelation(origin, target, relationType))
return;
return false;

if (relationCausesCycle(origin, target, relationType))
return;
return false;

AXID originID = origin->objectID();
AXID targetID = target->objectID();
Expand Down Expand Up @@ -4781,9 +4782,11 @@ void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject
}
#endif
}

return true;
}

void AXObjectCache::removeRelations(AXID axID)
void AXObjectCache::removeAllRelations(AXID axID)
{
AXTRACE("AXObjectCache::removeRelations"_s + " for axID " + axID.loggingString());

Expand All @@ -4805,29 +4808,32 @@ void AXObjectCache::removeRelations(AXID axID)
dirtyIsolatedTreeRelations();
}

void AXObjectCache::removeRelations(Element& origin, AXRelationType relationType)
bool AXObjectCache::removeRelation(Element& origin, AXRelationType relationType)
{
AXTRACE("AXObjectCache::removeRelations"_s + " for " + origin.debugDescription());
AXLOG(relationType);

auto* object = get(&origin);
if (!object)
return;
return false;

auto relationsIterator = m_relations.find(object->objectID());
if (relationsIterator == m_relations.end())
return;
return false;

auto targetIDs = relationsIterator->value.take(enumToUnderlyingType(relationType));
auto symmetric = symmetricRelation(relationType);
if (symmetric == AXRelationType::None)
return;
bool removedRelation = !targetIDs.isEmpty();

for (AXID targetID : targetIDs)
removeRelationByID(targetID, object->objectID(), symmetric);
auto symmetric = symmetricRelation(relationType);
if (symmetric != AXRelationType::None) {
for (AXID targetID : targetIDs)
removeRelationByID(targetID, object->objectID(), symmetric);
}

if (!targetIDs.isEmpty() && relationType == AXRelationType::OwnerFor)
if (removedRelation && relationType == AXRelationType::OwnerFor)
childrenChanged(object);

return removedRelation;
}

void AXObjectCache::removeRelationByID(AXID originID, AXID targetID, AXRelationType relationType)
Expand Down Expand Up @@ -4871,44 +4877,47 @@ void AXObjectCache::updateRelationsForTree(ContainerNode& rootNode)
}

for (const auto& attribute : relationAttributes())
addRelations(element, attribute);
addRelation(element, attribute);

// In addition to ARIA specified relations, there may be other relevant relations.
// For instance, LabelFor in HTMLLabelElements.
addLabelForRelation(element);
}
}

void AXObjectCache::addRelations(Element& origin, const QualifiedName& attribute)
bool AXObjectCache::addRelation(Element& origin, const QualifiedName& attribute)
{
if (attribute == aria_labeledbyAttr && origin.hasAttribute(aria_labelledbyAttr)) {
// The attribute name with British spelling should override the one with American spelling.
return;
return false;
}

bool addedRelation = false;
auto relationType = attributeToRelationType(attribute);
if (m_document->settings().ariaReflectionForElementReferencesEnabled()) {
if (Element::isElementReflectionAttribute(m_document->settings(), attribute)) {
if (auto reflectedElement = origin.getElementAttribute(attribute)) {
addRelation(&origin, reflectedElement.get(), relationType);
return;
}
if (auto reflectedElement = origin.getElementAttribute(attribute))
return addRelation(&origin, reflectedElement.get(), relationType);
} else if (Element::isElementsArrayReflectionAttribute(m_document->settings(), attribute)) {
if (auto reflectedElements = origin.getElementsArrayAttribute(attribute)) {
for (auto reflectedElement : reflectedElements.value())
addRelation(&origin, reflectedElement.get(), relationType);
return;
for (auto reflectedElement : reflectedElements.value()) {
if (addRelation(&origin, reflectedElement.get(), relationType))
addedRelation = true;
}
return addedRelation;
}
}
}

auto& value = origin.attributeWithoutSynchronization(attribute);
if (value.isNull()) {
if (auto* defaultARIA = origin.customElementDefaultARIAIfExists()) {
for (auto& target : defaultARIA->elementsForAttribute(origin, attribute))
addRelation(&origin, target.get(), relationType);
for (auto& target : defaultARIA->elementsForAttribute(origin, attribute)) {
if (addRelation(&origin, target.get(), relationType))
addedRelation = true;
}
}
return;
return addedRelation;
}

SpaceSplitString ids(value, SpaceSplitString::ShouldFoldCase::No);
Expand All @@ -4917,25 +4926,29 @@ void AXObjectCache::addRelations(Element& origin, const QualifiedName& attribute
if (!target || target == &origin)
continue;

addRelation(&origin, target.get(), relationType);
if (addRelation(&origin, target.get(), relationType))
addedRelation = true;
}

return addedRelation;
}

void AXObjectCache::addLabelForRelation(Element& origin)
{
bool addedRelation = false;

// LabelFor relations are established for <label for=...> and for <figcaption> elements.
if (RefPtr label = dynamicDowncast<HTMLLabelElement>(origin)) {
if (auto control = Accessibility::controlForLabelElement(*label)) {
addRelation(&origin, control.get(), AXRelationType::LabelFor);
dirtyIsolatedTreeRelations();
}
if (auto control = Accessibility::controlForLabelElement(*label))
addedRelation = addRelation(&origin, control.get(), AXRelationType::LabelFor);
} else if (origin.hasTagName(figcaptionTag)) {
RefPtr parent = origin.parentNode();
if (parent && parent->hasTagName(figureTag)) {
addRelation(getOrCreate(&origin), getOrCreate(parent.get()), AXRelationType::LabelFor);
dirtyIsolatedTreeRelations();
}
if (parent && parent->hasTagName(figureTag))
addedRelation = addRelation(getOrCreate(&origin), getOrCreate(parent.get()), AXRelationType::LabelFor);
}

if (addedRelation)
dirtyIsolatedTreeRelations();
}

void AXObjectCache::updateRelations(Element& origin, const QualifiedName& attribute)
Expand All @@ -4953,9 +4966,10 @@ void AXObjectCache::updateRelations(Element& origin, const QualifiedName& attrib
if (UNLIKELY(attribute == popovertargetAttr) && !is<HTMLInputElement>(origin) && !is<HTMLButtonElement>(origin))
return;

removeRelations(origin, relationType);
addRelations(origin, attribute);
dirtyIsolatedTreeRelations();
bool changedRelation = removeRelation(origin, relationType);
changedRelation |= addRelation(origin, attribute);
if (changedRelation)
dirtyIsolatedTreeRelations();
}

void AXObjectCache::relationsNeedUpdate(bool needUpdate)
Expand Down
12 changes: 6 additions & 6 deletions Source/WebCore/accessibility/AXObjectCache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,13 +640,13 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
static AXRelationType attributeToRelationType(const QualifiedName&);
enum class AddSymmetricRelation : bool { No, Yes };
static AXRelationType symmetricRelation(AXRelationType);
void addRelation(Element*, Element*, AXRelationType);
void addRelation(AccessibilityObject*, AccessibilityObject*, AXRelationType, AddSymmetricRelation = AddSymmetricRelation::Yes);
void removeRelationByID(AXID originID, AXID targetID, AXRelationType);
void addRelations(Element&, const QualifiedName&);
bool addRelation(Element*, Element*, AXRelationType);
bool addRelation(AccessibilityObject*, AccessibilityObject*, AXRelationType, AddSymmetricRelation = AddSymmetricRelation::Yes);
bool addRelation(Element&, const QualifiedName&);
void addLabelForRelation(Element&);
void removeRelations(Element&, AXRelationType);
void removeRelations(AXID);
bool removeRelation(Element&, AXRelationType);
void removeAllRelations(AXID);
void removeRelationByID(AXID originID, AXID targetID, AXRelationType);
void updateLabelFor(HTMLLabelElement&);
void updateLabeledBy(Element*);
void updateRelationsIfNeeded();
Expand Down

0 comments on commit 36de400

Please sign in to comment.