AX: <label>s that are targets of aria-labelledby lose their labelfor relationship when they change

Joshua Hoffman · hoffmanjoshua · commit f96d5091cd43 · 2025-09-16T07:07:39.000-07:00
https://bugs.webkit.org/show_bug.cgi?id=298891 rdar://158906980 Reviewed by Tyler Wilcock. When <label> elements change, the handleLabelChanged method of AXObjectCache get's called to update its labelfor relationships. In this code path, we clear all LabelFor relationships and re-add them based on the label's `for` attribute. This is fine when a control's label just uses the `for` attribute, but if the control itself uses aria-labelledby to point to the label element, we end up clearing this relationship. This causes us a control's label to be stale, for example, because a text change on a label won't know what other elements to update. Fix this by preventing clearing LabelFor relationships when the control uses aria-label or aria-labelledby. Test: accessibility/label-for-with-aria-labelledby.html Test: accessibility/label-for-with-aria-labelledby.html * LayoutTests/accessibility/label-for-with-aria-labelledby-expected.txt: Added. * LayoutTests/accessibility/label-for-with-aria-labelledby.html: Added. * LayoutTests/platform/glib/accessibility/label-for-with-aria-labelledby-expected.txt: Added. * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::handleAttributeChange): (WebCore::hasAnyARIALabelling): (WebCore::AXObjectCache::handleLabelChanged): (WebCore::AXObjectCache::updateLabelFor): (WebCore::AXObjectCache::addRelation): * Source/WebCore/accessibility/AXObjectCache.h: Canonical link: https://commits.webkit.org/300029@main
diff --git a/LayoutTests/accessibility/label-for-with-aria-labelledby-expected.txt b/LayoutTests/accessibility/label-for-with-aria-labelledby-expected.txt
@@ -0,0 +1,14 @@
+This tests that the aria-labelledby pointing to a label works when the label has a `for` attribute.
+
+	AXTitle: Hello
+	AXDescription: Hello
+	AXHelp:
+Changing inner HTML of label:
+	AXTitle: World
+	AXDescription: World
+	AXHelp:
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+ World
diff --git a/LayoutTests/accessibility/label-for-with-aria-labelledby.html b/LayoutTests/accessibility/label-for-with-aria-labelledby.html
@@ -0,0 +1,35 @@
+<!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>
+
+<input id="textbox" type="text" size=20 aria-labelledby="textbox-label">
+<label id="textbox-label" for="textbox">Hello</label>
+
+<script>
+let output = "This tests that the aria-labelledby pointing to a label works when the label has a `for` attribute.\n\n";
+
+var platformText;
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+    var textbox = accessibilityController.accessibleElementById("textbox");
+    output += `${platformTextAlternatives(textbox)}\n`;
+
+    document.getElementById("textbox-label").innerHTML = "World";
+    output += "Changing inner HTML of label:\n";
+    setTimeout(async function() {
+        await waitFor(() => {
+            platformText = platformTextAlternatives(textbox);
+            return platformText.includes("World");
+        });
+        output += `${platformTextAlternatives(textbox)}\n`;
+        debug(output);
+        finishJSTest();
+    }, 0);
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/glib/accessibility/label-for-with-aria-labelledby-expected.txt b/LayoutTests/platform/glib/accessibility/label-for-with-aria-labelledby-expected.txt
@@ -0,0 +1,12 @@
+This tests that the aria-labelledby pointing to a label works when the label has a `for` attribute.
+
+	AXTitle: Hello
+	AXDescription:
+Changing inner HTML of label:
+	AXTitle: World
+	AXDescription:
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+ World
diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -2873,12 +2873,14 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
         postNotification(element, AXNotification::DisabledStateChanged);
     else if (attrName == forAttr) {
         if (RefPtr label = dynamicDowncast<HTMLLabelElement>(element)) {
-            updateLabelFor(*label);
+            bool updatedLabelFor = updateLabelFor(*label);
 
-            if (RefPtr oldControl = element->treeScope().elementByIdResolvingReferenceTarget(oldValue))
-                postNotification(oldControl.get(), AXNotification::TextChanged);
-            if (RefPtr newControl = element->treeScope().elementByIdResolvingReferenceTarget(newValue))
-                postNotification(newControl.get(), AXNotification::TextChanged);
+            if (updatedLabelFor) {
+                if (RefPtr oldControl = element->treeScope().elementByIdResolvingReferenceTarget(oldValue))
+                    postNotification(oldControl.get(), AXNotification::TextChanged);
+                if (RefPtr newControl = element->treeScope().elementByIdResolvingReferenceTarget(newValue))
+                    postNotification(newControl.get(), AXNotification::TextChanged);
+            }
         }
     } else if (attrName == requiredAttr)
         postNotification(element, AXNotification::RequiredStatusChanged);
@@ -3106,16 +3108,25 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
         handleInputTypeChanged(*element);
 }
 
+static bool hasAnyARIALabelling(Element& element)
+{
+    return element.hasAttributeWithoutSynchronization(aria_labelAttr)
+        || element.hasAttributeWithoutSynchronization(aria_labelledbyAttr)
+        || element.hasAttributeWithoutSynchronization(aria_labeledbyAttr);
+}
+
 void AXObjectCache::handleLabelChanged(AccessibilityObject* object)
 {
     AXTRACE("AXObjectCache::handleLabelChanged"_s);
 
     if (!object)
         return;
 
+    bool updatedLabelFor = false;
     if (RefPtr label = dynamicDowncast<HTMLLabelElement>(object->element()))
-        updateLabelFor(*label);
-    else {
+        updatedLabelFor = updateLabelFor(*label);
+
+    if (!updatedLabelFor) {
         auto labeledObjects = object->labelForObjects();
         for (auto& labeledObject : labeledObjects) {
             updateLabeledBy(RefPtr { labeledObject->element() }.get());
@@ -3126,10 +3137,16 @@ void AXObjectCache::handleLabelChanged(AccessibilityObject* object)
     postNotification(object, protectedDocument().get(), AXNotification::LabelChanged);
 }
 
-void AXObjectCache::updateLabelFor(HTMLLabelElement& label)
+bool AXObjectCache::updateLabelFor(HTMLLabelElement& label)
 {
+    if (RefPtr control = Accessibility::controlForLabelElement(label)) {
+        if (hasAnyARIALabelling(*control))
+            return false;
+    }
+
     removeRelation(label, AXRelation::LabelFor);
     addLabelForRelation(label);
+    return true;
 }
 
 void AXObjectCache::updateLabeledBy(Element* element)
@@ -5341,9 +5358,7 @@ bool AXObjectCache::addRelation(Element& origin, Element& target, AXRelation rel
 
     if (relation == AXRelation::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))
+        if (hasAnyARIALabelling(target))
             return false;
     }
 
diff --git a/Source/WebCore/accessibility/AXObjectCache.h b/Source/WebCore/accessibility/AXObjectCache.h
@@ -728,7 +728,7 @@ class AXObjectCache final : public CanMakeWeakPtr<AXObjectCache>, public CanMake
     bool removeRelation(Element&, AXRelation);
     void removeAllRelations(AXID);
     void removeRelationByID(AXID originID, AXID targetID, AXRelation);
-    void updateLabelFor(HTMLLabelElement&);
+    bool updateLabelFor(HTMLLabelElement&);
     void updateLabeledBy(Element*);
     void updateRelationsIfNeeded();
     void updateRelationsForTree(ContainerNode&);
